How to handle multiple possible early returns from function without lots of duplicate checking code

Question

Given that I do not want to use exceptions in this code, how can I remove the duplicated:

if (rc != SQLITE_OK) {
    return rc;
}

checks after most statements in this function below?

int sqlite::update(const std::string& table_name, const std::vector<column_values>& fields, const std::vector<column_values>& where) {
    if (db_ == nullptr) {
        return SQLITE_ERROR;
    }
    
    const std::string sql = update_helper(table_name, fields, where);
    sqlite3_stmt* stmt = NULL;
    int rc = sqlite3_prepare_v2(db_, sql.c_str(), -1, &stmt, NULL);
    if(rc != SQLITE_OK) {
        return rc;
    }

    // loop thru each parameter, calling bind for each one
    rc = bind_fields(stmt, fields);
    if(rc != SQLITE_OK) {
        return rc;
    }

    // loop thru each where parameter, calling bind for each one
    rc = bind_where(stmt, where);
    if (rc != SQLITE_OK) {
        return rc;
    }

    return step_and_finalise(stmt);
}

Answer 1

First off, you are leaking the sqlite3_stmt if something goes wrong. You need to call sqlite3_finalize() before return'ing anything.

As for your question, you can wrap the duplicate if..returns in a preprocessor macro, eg:

#define CHECK_RC(rc) \
if (rc != SQLITE_OK) \
{ \
    sqlite3_finalize(stmt); \
    return rc; \
}

int sqlite::update(const std::string& table_name, const std::vector<column_values>& fields, const std::vector<column_values>& where) {
    if (db_ == nullptr) {
        return SQLITE_ERROR;
    }
    
    const std::string sql = update_helper(table_name, fields, where);
    sqlite3_stmt* stmt = NULL;

    int rc = sqlite3_prepare_v2(db_, sql.c_str(), -1, &stmt, NULL);
    CHECK_RC(rc);

    // loop thru each parameter, calling bind for each one
    rc = bind_fields(stmt, fields);
    CHECK_RC(rc);

    // loop thru each where parameter, calling bind for each one
    rc = bind_where(stmt, where);
    CHECK_RC(rc);

    return step_and_finalise(stmt);
}

Alternatively:

#define CHECK_RC(op) \
{ \
    int rc = op; \
    if (rc != SQLITE_OK) \
    { \
        sqlite3_finalize(stmt); \
        return rc; \
    } \
}

int sqlite::update(const std::string& table_name, const std::vector<column_values>& fields, const std::vector<column_values>& where) {
    if (db_ == nullptr) {
        return SQLITE_ERROR;
    }
    
    const std::string sql = update_helper(table_name, fields, where);
    sqlite3_stmt* stmt = NULL;

    CHECK_RC(sqlite3_prepare_v2(db_, sql.c_str(), -1, &stmt, NULL));

    // loop thru each parameter, calling bind for each one
    CHECK_RC(bind_fields(stmt, fields));

    // loop thru each where parameter, calling bind for each one
    CHECK_RC(bind_where(stmt, where));

    return step_and_finalise(stmt);
}

Or, you can re-write the code to not use multiple returns, eg:

int sqlite::update(const std::string& table_name, const std::vector<column_values>& fields, const std::vector<column_values>& where) {
    int rc;
    if (db_ == nullptr) {
        rc = SQLITE_ERROR;
    }
    else {    
        const std::string sql = update_helper(table_name, fields, where);
        sqlite3_stmt* stmt = NULL;

        rc = sqlite3_prepare_v2(db_, sql.c_str(), -1, &stmt, NULL);
        if (rc == SQLITE_OK) {
            // loop thru each parameter, calling bind for each one
            rc = bind_fields(stmt, fields);
            if (rc == SQLITE_OK) {
                // loop thru each where parameter, calling bind for each one
                rc = bind_where(stmt, where);
                if (rc == SQLITE_OK) {
                    rc = step_and_finalise(stmt);
                    stmt = NULL;
                }
            }
            sqlite3_finalize(stmt);
        }
    }
    return rc;
}

Or, you can throw an exception, eg:

void check_rc(int rc)
{
    if (rc != SQLITE_OK)
        throw rc;
}

void check_ptr(void* ptr)
{
    if (ptr == nullptr)
        check_rc(SQLITE_ERROR);
}

int sqlite::update(const std::string& table_name, const std::vector<column_values>& fields, const std::vector<column_values>& where) {
    try {
        check_ptr(db_);
    
        const std::string sql = update_helper(table_name, fields, where);
        sqlite3_stmt* stmt = NULL;

        check_rc(sqlite3_prepare_v2(db_, sql.c_str(), -1, &stmt, NULL));

        try {
            // loop thru each parameter, calling bind for each one
            check_rc(bind_fields(stmt, fields));

            // loop thru each where parameter, calling bind for each one
            check_rc(bind_where(stmt, where));

            return step_and_finalise(stmt);
        }
        catch (const int) {
            sqlite3_finalize(stmt);
            throw;
        }
    }
    catch (const int errCode) {
        return errCode;
    }
}

Answer 2

The advantage of this version is that it has no impact outside the method.

int sqlite::update(const std::string& table_name, const std::vector<column_values>& fields, const std::vector<column_values>& where) {

    auto check = [](int rc) { if (rc != SQLITE_OK) throw rc; };

    try
    {
        check(db_ == nullptr ? SQLITE_ERROR : SQLITE_OK);
        const std::string sql = update_helper(table_name, fields, where);
        sqlite3_stmt* stmt = NULL;

        check(sqlite3_prepare_v2(db_, sql.c_str(), -1, &stmt, NULL));

        // loop thru each parameter, calling bind for each one
        check(bind_fields(stmt, fields));

        // loop thru each where parameter, calling bind for each one
        check(bind_where(stmt, where));

        return step_and_finalise(stmt);
    }
    catch (const int rc)
    {
        return rc;
    }
}

Answer 3

In a linear sequence of operations like this, I approach it slightly differently than most:

Instead of checking for the success of the previous step, I check whether I have the prerequisites for the next step.

int sqlite::update(
    const std::string& table_name,
    const std::vector<column_values>& fields,
    const std::vector<column_values>& where
) {
    sqlite3_stmt* stmt = NULL;

    int rc = SQLITE_ERROR;
    if (db_ != nullptr) {
        const std::string sql = update_helper(table_name, fields, where);
        rc = sqlite3_prepare_v2(db_, sql.c_str(), -1, &stmt, NULL);
    }
    
    if (rc == SQLITE_OK) {
        // loop thru each parameter, calling bind for each one
        rc = bind_fields(stmt, fields);
    }

    if (rc == SQLITE_OK) {
        // loop thru each where parameter, calling bind for each one
        rc = bind_where(stmt, where);
    }

    if (rc == SQLITE_OK) {
        rc = step_and_finalise(stmt);
    }

    return rc;
}

Admittedly, this doesn't look much different than the original code. That's commonly the case when the prerequisite for each step is that all of the previous steps must have succeeded. In other examples, the checking doesn't look quite as redundant.

The advantage is that this is as linear as the early-return solution, without the confusion and complications sometimes caused by early returns. In the successful case, it does no more comparisons than necessary. And yet it doesn't have the deep nesting that is usually presented as the alternative to early returns.

The cost is that an error will involve some redundant checks. But that's a tiny cost unless errors are exceptionally common. And it's possible that the optimizer will eliminate them by creating early returns for you.

Answer 4

With external work, a monadic version can be written.

Pseudocode would look like:

expected<void, RC>  doStuff( arg1, arg2 ){
  auto a = op1(arg1, arg2 );
  return a->*op2(arg2)->*op3(arg1);
}

or

expected<void,RC> doStuff(arg1,arg2){
  auto a = co_await op1(arg1);
  return op2( co_await op3(arg2), co_await op4(arg1, arg2) );
}

but the boilerplate outside the function far exceeds the boilerplate removed inside the function.

Lastly, there are proposals to add in zero cost exceptions.

Those, and macros, are about your options if you want control flow to not require you to type the control flow.

For the expected case, the idea is that an error expected, when you do ->* on it, doesn't call the operation, but just passes through the error state.

It is a form of manual exceptions without exceptions, but there is no way to automate the boilerplate without, well, more boilerplate at the point of use or in wrapping code specific to the set of operations.

The coroutine one has unavoidable allocation overhead (well, your code cannot avoid it; in theory the compiler can), and some of the most arcane under the hood code. Again, here we are trying to fake exception like syntax without exceptions; the co_await when it gets an error, passes it through to the return value.

The TL;DR is "nope, you can't get rid of that control flow boilerplate".

Answer 5

You can use a lambda and if-initializer expressions to clean the code up, but you're still going to have to return the error code:

int sqlite::update(const std::string& table_name, const std::vector<column_values>& fields, const std::vector<column_values>& where) {
    if (db_ == nullptr) {
        return SQLITE_ERROR;
    }

    const std::string sql = update_helper(table_name, fields, where);
    sqlite3_stmt* stmt = NULL;

    
    const auto validate_sql = [](int rc) -> bool {
        return rc != SQLITE_OK;
    };
    if(int rc = sqlite3_prepare_v2(db_, sql.c_str(), -1, &stmt, NULL); validate_sql(rc)) {
        return rc;
    }

    // loop thru each parameter, calling bind for each one
    if(int rc = bind_fields(stmt, fields); validate_sql(rc)) {
        return rc;
    }

    // loop thru each where parameter, calling bind for each one
    if(int rc = bind_where(stmt, where); validate_sql(rc)) {
        return rc;
    }

    return step_and_finalise(stmt);
}