views:

658

answers:

6

Hi, everyone!

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?

A: 

How about making the conversion explicit?

Ashalynd
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.
atomice
A: 

I don't believe you can.

atomice
+2  A: 

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.

Therefor 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.

sbi
Not the point of the question. How can I write Fixed * 10 if my constructor is explicit??
SadSido
@SadSido: Sorry. I have added my opinion on that as an edit.
sbi
Thank you! Your edit is much more helpful (although a little bit pessimistic =)
SadSido
@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.
sbi
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 McCarthy
@Adrian: And why would that class need implicit conversion at all?
sbi
By the way, is it a big overhead to convert the operands to CFixed explicitly? I.e. always write: CFixed(0) * CFixed(10.0f) (granted, it doesn't look nice but at least you know what you are getting).
Ashalynd
@Ashalynd If it wasn't a big overhead, why would we need implicit conversions at all? Your code will not get prettier from static_cast<CFixed>(10) =)
SadSido
A: 

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.

Bill
And what will happen inside the templates? :-) Ẹxactly the same thing!
Pavel Shved
I think you caught me mid-edit. The same thing as what?
Bill
@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 }` Shouldn't that do what the OP wants?
sbi
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.
Pavel Minaev
@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 Minaev
@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?)
sbi
Asaf
The templates are almost near BUT... calling operator*( CFixed, CFixed ) fails, because there is ambiguity between operator*( TYPENAME, CFixed ) and operator* ( CFixed, TYPENAME )... And that cannot be specialized because there is an ambiguity of what template is being specialized...
SadSido
sbi
+4  A: 

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.

Pavel Shved
....and double?
UncleBens
@UncleBens , `double` will be converted to `float`.
Pavel Shved
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`
UncleBens
+1  A: 

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.

UncleBens
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...
SadSido
... looks like I am misunderstanding enable_if a little ...
SadSido
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.
UncleBens
Heh. It works! I guess it is much better than overloading the float operators (no need for code duplication)...
SadSido