8

I have a class that encapsulates some arithmetic, let's say fixed point calculations. I like the idea of overloading arithmetic operators, so I write the following:

class CFixed
{
   CFixed( int   );
   CFixed( float );
};

CFixed operator* ( const CFixed& a, const CFixed& b )
{ ... }

It all works. I can write 3 * CFixed(0) and CFixed(3) * 10.0f. But now I realize, I can implement operator* with an integer operand much more effective. So I overload it:

CFixed operator* ( const CFixed& a, int b )
{ ... }
CFixed operator* ( int a, const CFixed& b )
{ ... }

It still works, but now CFixed(0) * 10.0f calls overloaded version, converting float to int ( and I expected it to convert float to CFixed ). Of course, I can overload a float versions as well, but it seems a combinatorial explosion of code for me. Is there any workaround (or am I designing my class wrong)? How can I tell the compiler to call overloaded version of operator* ONLY with ints?

Improve this question
3
  • I don't believe you can. Commented Oct 22, 2009 at 14:41
  • 2
    On the other hand, constructors taking a single parameter (and a fortiori a built-in) should probably be declared explicit, which would of course prevent promotion.... but also prevent bugs. Commented Oct 22, 2009 at 18:15
  • 1
    Another "explicit" advocate =) Implicit constructors are helpful, provided you realize what they are doing. It is great to design a function, accepting CFixed argument and pass integers there! Commented Oct 23, 2009 at 6:48

5 Answers 5

Reset to default
5

You should overload with float type as well. Conversion from int to user-specified type (CFixed) is of lower priority than built-in floating-integral conversion to float. So the compiler will always choose function with int, unless you add function with float as well.

For more details, read 13.3 section of C++03 standard. Feel the pain.

It seems that I've lost track of it too. :-( UncleBens reports that adding float only doesn't solve the problem, as version with double should be added as well. But in any case adding several operators related to built-in types is tedious, but doesn't result in a combinatorial boost.

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

@UncleBens , double will be converted to float.
It says "ambiguous overload" to me. Trying CFixed(0) * 1.0, Comeau informs me in particular that both operator*(CFixed, int) and operator*(CFixed, float) are equal candidates for CFixed * double
3

If you have constructors which can be invoked with just one argument, you effectively created an implicit conversion operator. In your example, wherever a CFixed is needed, both an int and a float can be passed. This is of course dangerous, because the compiler might silently generate code calling the wrong function instead of barking at you when you forgot to include some function's declaration.

Therefore a good rule of thumb says that, whenever you're writing constructors that can be called with just one argument (note that this one foo(int i, bool b = false) can be called with one argument, too, even though it takes two arguments), you should make that constructor explicit, unless you really want implicit conversion to kick in. explicit constructors are not used by the compiler for implicit conversions.

You would have to change your class to this: 

class CFixed
{
   explicit CFixed( int   );
   explicit CFixed( float );
};

I have found that there are very few exceptions to this rule. (std::string::string(const char*) is a rather famous one.)

Edit: I'm sorry, I missed the point about not allowing implicit conversions from int to float.

The only way I see to prevent this is to provide the operators for float as well.

Improve this answer

7 Comments

Not the point of the question. How can I write Fixed * 10 if my constructor is explicit??
Thank you! Your edit is much more helpful (although a little bit pessimistic =)
@SadSido: It might be that Bill has found a solution, although Pavel is right in that, in its current form, this will simply produce compile errors.
Actually, this answer is almost right. You can leave the implicit constructor for float and just make the int one explicit. Since you've written overrides for the int cases, you don't really need the implicit conversions for those. This would eliminate the ambiguity.
@Adrian: And why would that class need implicit conversion at all?
|
2

Assuming you'd like the specialized version to be picked for any integral type (and not just int in particular, one thing you could do is provide that as a template function and use Boost.EnableIf to remove those overloads from the available overload set, if the operand is not an integral type.

#include <cstdio>
#include <boost/utility/enable_if.hpp>
#include <boost/type_traits/is_integral.hpp>

class CFixed
{
public:
   CFixed( int   ) {}
   CFixed( float ) {}
};

CFixed operator* ( const CFixed& a, const CFixed&  )
{ puts("General CFixed * CFixed"); return a; }

template <class T>
typename boost::enable_if<boost::is_integral<T>, CFixed>::type operator* ( const CFixed& a, T  )
{ puts("CFixed * [integer type]"); return a; }

template <class T>
typename boost::enable_if<boost::is_integral<T>, CFixed>::type operator* ( T , const CFixed& b )
{ puts("[integer type] * CFixed"); return b; }


int main()
{
    CFixed(0) * 10.0f;
    5 * CFixed(20.4f);
    3.2f * CFixed(10);
    CFixed(1) * 100u;
}

Naturally, you could also use a different condition to make those overloads available only if T=int: typename boost::enable_if<boost::is_same<T, int>, CFixed>::type ...

As to designing the class, perhaps you could rely on templates more. E.g, the constructor could be a template, and again, should you need to distinguish between integral and real types, it should be possible to employ this technique.

Improve this answer

3 Comments

I haven't tried this yet, but shouldn't compiler prefer non-template versions of the function prior to template ones? I think your "main" will call a "General CFixed" version 4 times...
It will prefer templates if they have a better match. enable_if makes the overloads candidates only if the condition is met, and in case of float only the first overload is the only one left to choose from.
Heh. It works! I guess it is much better than overloading the float operators (no need for code duplication)...
0

How about making the conversion explicit?

Improve this answer

1 Comment

How would that help? The poster wanted CFixed(float) to be called if a float was passed to operator * - adding the explicit keyword will make that even less likely.
0

Agree with sbi, you should definitely make your single-parameter constructors explicit.

You can avoid an explosion in the operator<> functions you write with templates, however:

template <class T>
CFixed operator* ( const CFixed& a, T b ) 
{ ... } 

template <class T>
CFixed operator* ( T a, const CFixed& b ) 
{ ... }

Depending on what code is in the functions, this will only compile with types that you support converting from.

Improve this answer

8 Comments

And what will happen inside the templates? :-) Ẹxactly the same thing!
@Pavel: Actually, I think Bill is right (even thought he might not have realized it :) ). Look at this: template <class T> CFixed operator* ( const CFixed& a, T b ) { return a*CFixed(b); } Shouldn't that do what the OP wants?
This is a bad idea, because such templates will be considered for overloading for any call to operator*, where one of the arguments is of type CFixed or is convertible to it, even when T isn't actually a type for which * with CFixed is available - it can still pick that overload, and then fail when trying to instantiate it. Since the error is going to be within the body of operator*, SFINAE won't kick in.
@sbi: it does what OP wants, but it also creates a CFixed temporary, which is what he wants to avoid. He originally started with a single operator*(CFixed, CFixed), which actually does everything he wants already; but he can implement operator*(CFixed, int) more efficiently if he knows it's an int in advance, which is why he wants a separate operator for that. This doesn't solve that problem.
@Pavel: I thought so, too, at first (see my answer), but now I think, in order to prevent the combinatorial explosion, he would rather suffer the temporary. Of course, this still leaves the problem of SFINAE not kicking in. (Maybe this could be taken care of by some default template argument?)
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.