Does this function have explicit return values ​​on all control paths?

Question

I have a unit-oriented Heaviside step function for any data type that I encoded with:

template <typename T> int h1(const T& t){ if (t < 1){ return 0; } else if (t >= 1){ return 1; } } 

In a code review, my browser told me that there is no explicit return on all control paths. And the compiler does not warn me either. But I do not agree; conditions are mutually exclusive. How can I handle this?

Answer 1

It depends on how the template is used. For int you are fine.

But , if t is an IEEE754 double floating-point type with a value set to NaN , neither t < 1 nor t >= 1 are true and therefore program control reaches the end of the if ! This causes the function to return without an explicit value; whose behavior is undefined.

(In the more general case, when t overloads the < and >= operators in such a way as to not cover all the possibilities, program control will reach the end of the if block without an explicit return .)

The moral of this story here is to decide which branch should be defaulted, and make this case else .

Answer 2

Just because the code is correct does not mean that it could not be better. Proper execution is the first step in quality, not the last.

 if (t < 1) { return 0; } else if (t >= 1){ return 1; } 

The above option is “correct” for any data type t than for normal behavior for < and >= . But this:

 if (t < 1) { return 0; } return 1; 

It is easier to see by checking that each case is covered, and avoids the second unnecessary comparison at all (that some compilers may not be optimized). The code is not only read by compilers, but also by people, including you in 10 years. Give people a break and write just for their understanding.

Answer 3

As already noted, some special numbers can be either < or >= , so your reviewer is just right.

Question: what made you want to encode it that way in the first place? Why do you even think that life is so hard for yourself and for others (people who need to maintain your code)? Just the fact that you are smart enough to infer that < and >= should cover all cases does not mean that you need to make the code more complex than necessary. As for physics, for code too: make everything as simple as possible, but not simpler (I believe Einstein said that).

Think about it. What are you trying to achieve? There should be something like this: "Return 0 if the input is less than 1, otherwise return 1." What you did was add intelligence by saying ... oh, but that means I return 1 if t is greater than or equal to 1. This kind of unnecessary "x implies y" requires additional thought work on behalf of the maintainer. If you think this is good, I would advise doing a couple of years of code maintenance on your own.

If this were my review, I would make one more remark. If you use the if statement, you can basically do whatever you want in all branches. But in this case you are not doing anything. All you want to do is return 0 or 1 depending on whether t <1 or not. In such cases, I think the expression ??: is much better and readable than the if . In this way:

return t<1 ? 0 : 1;

I know that the operator ?: prohibited in some companies, and I think this is terrible. ?: Usually better complies with specifications, and it can make the code much easier to read (if used with caution) ...