CODEWITHSUNDEEP
c++coding-style
Nasr
Nasr

Reputation: 2632

c++ Same checking on function return over and over again

int cmd_x() {
    int result;
    result = func1(x, y);
    if (result != 0) {
        return result;
    }

    result = func2();
    if (result != 0) {
        return result;
    }

    result = func1(x, y, z, w);
    if (result != 0) {
        return result;
    }

    ...

    return result;
}

cmd_x executes multiple functions on waterfall way. Each function returns result. I should make sure that result is success to continue to the next step.

This if condition appears multiple times on the code, which makes it harder to follow and read.

Is there a way to get rid of this condition cleanly?

I was thinking of creating a array of function pointers and loop over it to check the code only once, but i couldn't implement it since number of function arguments are different.

Upvotes: 0

Views: 488

Answers (2)

R Sahu
R Sahu

Reputation: 206567

I can think of the following ways to simplify your code a bit.

Option 1: Use conditional expressions

int cmd_x()
{
    int x = 0, y = 0, z = 0, w = 0;

    int result = func1(x, y);

    result = (result != 0) ? result : func2();

    result = (result != 0) ? result : func1(x, y, z, w);

    // ...

    result = (result != 0) ? result : funcN();

    return result;
}

Option 2: Use a std::vector of std::functions

int cmd_x()
{
    int x = 0, y = 0, z = 0, w = 0;
    std::vector<std::function<int()>> functionList =
    {
        // Let the lambda functions capture what they need 
        [x, y]() -> int { return func1(x, y); },
        [] () -> int { return func2(); },
        [x, y, z, w] () -> int { return func1(x, y, z, w); },
        [] () -> int { return funcN(); }
    };

    for ( auto fn : functionList )
    {
        int result = fn();
        if ( result != 0 )
        {
            return result;
        }
    }

    return 0;
}

Option 3: Use a helper function and std::function

This is a hybrid approach that uses the conditional expression in the helper function and lambda functions in the main function.

int cmd_helper(int r, std::function<int()> fn)
{
   return ( r != 0 ) ? r : fn();
}

int cmd_x()
{
    int x = 0, y = 0, z = 0, w = 0;

    int result = 0;

    result = cmd_helper(result, [x, y]() -> int { return func1(x, y); });

    result = cmd_helper(result, [] () -> int { return func2(); });

    result = cmd_helper(result, [x, y, z, w] () -> int { return func1(x, y, z, w); });

    result = cmd_helper(result, [] () -> int { return funcN(); });

    return result;
}

Upvotes: 1

Max Langhof
Max Langhof

Reputation: 23681

This looks a lot like error handling ("if I get a nonzero error code, stop and return that code"). If these errors are exceptional, then it may make sense to use exception handling within func1 etc. - if they would return a nonzero error code, they instead throw an expection. This exception can then be caught at the most appropriate place (which may be several function calls up), saving you from doing error handling all the way through the call hierarchy.

If the errors are not exceptional, things get complicated. If you can pack all your function calls into some sort of container, you could iterate over that container. The problem here would be finding a common type for the functions.
Alternatively, variadic templates could do the job:

template<class Func, class... OtherFuncs>
int cmd_x_impl(Func&& func, OtherFuncs&&... otherFuncs)
{
   int result = func();
   if (result != 0)
     return result;
   cmd_x_impl(otherFuncs...);
}

template<class Func>
int cmd_x_impl(Func&& func)
{
    return func();
}

int cmd_x() {
    return cmd_x_impl(
        [&]() { return func1(x, y); },
        [&]() { return func2(); },
        [&]() { return func1(x, y, z, w); }
    );
}

https://godbolt.org/g/wihtCj

This wraps all your function calls in lambdas, and then uses variadic template recursion to call them one after the other as long as each result is 0. The advantage over the std::function approach shown in another answer is that this is much easier for the compiler to see through and optimize. It is also more concise in the usage and more reusable.

Upvotes: 2

Related Questions