Passing argument to a function

Is it a bad programming principle to pass arg to a function which isn't "exact match" type wise but has trivial or user defined conversion to this type? For example:

void f(bool option);

and later in main (this is high开发者_如何学编程ly hypothetical so please don't give advise on that code):

int a = getSomeValue();

f(a);//here instead of bool I'm passing int - is it a bad programming practice or not?

I think most people will consider this bad practice. Writing f(a !=0) is much clearer and expresses the intent concisely.

Conversion to bool is a bit of a special case, because it doesn't behave like integer conversions, so a lot of people like to see it explicit, and done with != 0 rather than a cast.

If you're passing an int to a function that takes long, then I certainly wouldn't want to see that cluttered with any kind of explicit conversion. Similarly I wouldn't want to see char* cast to const char* in order to pass it to strlen. Those conversions "don't really do anything": they always preserve the value of the input, and in the case of the pointer it doesn't let you do anything with the referand that you couldn't do using the original pointer.

If you're passing an unsigned long to a function that takes unsigned int, then on some implementations (but by no means on all common ones), it's a "narrowing conversion" which depending on the values can discard information by taking a modulus. It's often a good idea to make the conversion explicit in such cases, so that the casual reader doesn't have to check the function signature to realise that it happens. These conversions "do something" that might be surprising.

The same goes when converting between integer and floating types: although int -> double is harmless on pretty much any implementation you'll ever see, double -> int has undefined behavior for some values, so you don't want a reader to miss it.

Sometimes however, quite complex user-defined conversions are provided specifically so that you can call a function and benefit from an implicit conversion. So for those, you should probably follow the usage examples in whatever API defined the conversion.

For example if Foo has an implicit constructor that takes a std::string, and some function bar takes a Foo, that's not a whole lot different from the POV of a caller of bar than if the API had provided only an explicit conversion but two overloads of bar, one taking Foo and one that takes std::string but converts it straight to Foo and calls the other. So relying on the conversion in a case like that is no worse than overloading a function. The latter is not regarded as the crime of the century. Except by Python programmers.

Yes, I think this is bad programming practice.

Relying on implicit conversions may lead to very strange errors. I once had the following problem. It all started with a function like this:

void doSomething (ClassX x, ClassY y, bool b);

This function was called with code like this:

ClassX x;
ClassY y;
doSomething (x, y, true);

Later, somebody decided that the arguments of doSomething had to be changed, since doSomething now also required an instance of ClassZ. Also, the boolean argument was true in many cases, so it was changed into a default argument, like this:

void doSomething (ClassX x, ClassY y, ClassZ z, bool b=true);

Although most of the code was changed, the call shown before, wasn't changed. Strange enough, ClassZ had a non-explicit constructor taking a bool as argument:

class ClassZ
   {
   public:
      ClassZ(bool b);
   };

What happened was the following. This call:

ClassX x;
ClassY y;
doSomething (x, y, true);

Was silently transformed into this by the compiler:

ClassX x;
ClassY y;
doSomething (x, y, ClassZ(true), true);

With the 4th argument being the default true.

Years later, we found out that something didn't work correctly, and after a lot of debugging, we found out that this call was the cause of the problem.

Therefore:

• never rely on implicit conversions
• mark constructors with 1 argument as explicit
• try to avoid the use of default arguments

It all depends on context.

In this very specific situation that you site in your question. I prefer the explicit test rather than than the implicit conversion of 0 to false (But that is also because the names of the variables and the function are so meaningless if they were named differently then I may not mind).

But there are other situations were the implicit conversion is fine:

std::ifstream f("File");
if (f) 
{
    // Testing f is relatively standard.
}

Any time there is information loss. I would expect an explicit cast with a comment explaining why the information loss is OK.

long theDistanceToLodond = getDistance("ToLondon");
InsideM25Action(static_cast<short>(theDistanceToLondon)); // All distances inside the M25 fit in a short therefore no information loss.

But casts where no information is lost should not have an explicit cast (it over-crowds the code without providing any benifit).I would not expect explicit down casts of pointers towards the base class etc.

If you pass short variable to f(int), then it's fine. But the opposite is dangerous (i.e int to short - possible overflow!), and most compilers wouldn't be happy and so would give you warnings.

As for f(bool), so yes, the intent is more clear when you write f(a!=0) when calling f. Here we do it for the sake of readability, not that it's dangerous to pass simply a as f(a).

Since you're using C++, you should have done:

   int a = getSomeValue();
   f(static_cast<bool>(a));

That will surely express what is your intention, that's why we have many types of explicit casts.