Question

When refactoring some code, I often encounter this :

bool highLevelFunc foo()
{
  // ...
  bool result = LesserLevelFunc();
    if (!result) return false;
  // ... Keep having fun if we didn't return
}

Is there any way to make this a little more sexy and less verbose ? Without any overhead or pitfall of course.

I can think of a macro

#define FORWARD_IF_FALSE(r) if (!r) return r;

bool highLevelFunc foo()
{
  // ...
  FORWARD_IF_FALSE(LesserLevelFunc());
  // ...
}

Anything better, i.e without preprocessor macro?

Was it helpful?

Solution

To me, "readable" code is sexy. I find the original code more readable than your proposal, since the original uses standard C++ syntax and the latter uses a macro which I'd have to go and look up.

If you want to be more explicit, you could say if (result == false) (or better yet, if (false == result) to prevent a possible assignment-as-comparison bug) but understanding the ! operator is a fairly reasonable expectation in my opinion.

That said, there is no reason to assign the return value to a temporary variable; you could just as easily say:

if (!LesserLevelFunc()) return false;

This is quite readable to me.

EDIT: You could also consider using exceptions instead of return values to communicate failure. If LesserLevelFunc() threw an exception, you would not need to write any special code in highLevelFunc() to check for success. The exception would propagate up through the caller to the nearest matching catch block.

OTHER TIPS

Because you might be continuing if LesserLevelFunc returns true, I suggest keeping it pretty close to how it is now:

if (!LesserLevelFunc())
    return false;

First of all introducing the macro you are making the code unsafe. Moreover your macro is invalid.

The expression after the negation operator shall be enclosed in parentheses.

#define FORWARD_IF_FALSE(r) if (!( r ) ) return r;

Secondly the macro calls r twice. Sometimes two calls of a function is not equivalent to one call of the same function. For example the function can have some side effects or internal flags that are switched on/off in each call of the function.

So I would keep the code as is without introducing the macro because the macro does not equivalent to the symantic of the original code.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top