CODEWITHSUNDEEP
c++optimizationcode-complexity
Mariners
Mariners

Reputation: 509

Does replacing repititve "if" block with a macro reduce code complexity in C++?

I had a question. I need to refactor a part of my code to reduce complexity. I have multiple similar "if" constructs which repeats after every some random utility API call, based on which I need to rollback the transaction done or else continue. I was just wondering whether replacing the "if" construct would help me reducing the complexity. I know macro expands in the code and that is why I am not sure whether this approach would help me. I am fairly new at refactoring codes. If someone with an idea and knowledge on refactoring could suggest me some way, it would be really helpful. It is like replacing below code

retCode = certman_set_cmp_server_psk_refnum(ctx,(char*)domain.c_str(),NULL,NULL,0);
if (retCode != 0)
{
    if(startedTxn == true)
    {
        status = ldapInterface.rollback_transaction();
        if(0 != status)
        {
            clifwk_trace(session, "Rollback failed in internal transaction", CLIFWK_DEBUG);
            syslog(LOG_ERR, "Rollback failed in internal transaction");
            return CERTMAN_COMMIT_TRANSACTION_FAILED;
        }
        DEL_IF_NOT_NULL (ctx);
    }
    return retCode;
}

with retCode = certman_set_cmp_server_ip_port(ctx,(char*)domain.c_str(),NULL,0); CHECK_ROLLBACK_TRANSACTION(session,ctx,ldapInterface,retCode,startedTxn); RETURN_IF_ERROR(retCode);

Will this approach work for me to reduce the complexity ?

Upvotes: 0

Views: 107

Answers (1)

MSalters
MSalters

Reputation: 179907

Your complexity problem appears to be caused by too much C, too little C++. Adding more C in the form of macro's only makes it worse.

For instance, DEL_IF_NOT_NULL (ctx); is not one but two three flags. I assume it's a if (ctx) delete ctx construct. The first red flag is the if statement, which doesn't do anything. The second red flag is that you're using raw pointers, instead of relying on smart pointers. This would have also fixed the third red flag, and that's the failure to clean up in the 0!=status error case.

Similar complexicy is caused by using retcode instead of exceptions.

This is how simple the code should be:

std::unique_ptr<T> ctx = foo();
auto transaction = ldapInterface.get_transaction(); // will rollback unless committed
certman::set_cmp_server_psk_refnum(*ctx,domain,nullptr,nullptr,0);
transaction.commit();

Upvotes: 3

Related Questions