Monday, May 3, 2010

Refactoring - Nested Code

Its the dream of every programmer to pen down code that reflects their prowess, but at times this very prowess could end up churning out cryptic\crude coding styles. So, lets see some common practices where refactoring could have scored up on readability and maintainability factors.

Before we begin with, its imperative to ask; What is "Refactoring" all about?. In short, its all about pruning and grooming existing code into a well structured and well laid format for improved readability and maintainability.
Here is a sample bloated code snippet that badly needs refactoring. In this snippet an imaginary procedure named "ExecOps" is to be called; but before this, some conditions had to evaluated in an hierarchical pattern and some logging stuff too.

if(ConditionA())
{
    Log("ConditionA success");
 
    if (ConditionB())
    {
        Log("ConditionB success");

        if (ConditionC())
        {
            Log("ConditionC success");

            if (ConditionD())
            {
                Log("ConditionD success");
                ExecOps();
            }
            else
            {
                Log("ConditionD Failed");
            }
        }
        else
        {
             Log("ConditionC Failed");
        }
    }
    else
    {
         Log("ConditionB Failed");
    }   
}
else
{
    Log("ConditionA Failed");
}
huh, a pretty deep nested stuff? ok. Seems a question might be lurking in your mind like, why not move all those log calls to those conditions(ConditionA(), ConditionB() etc.); Good question. Let me ask you, what if you are bound from amending those existing conditions(Conditional procedures) or what if, the conditions are from an object's instance to which you don't have access to; either way its going to be risky affair. Please note, amending existing code without proper know-how could end up breaking the system or the build itself. So lets position ourselves on the safer side, by keeping away from making those changes to those existing stuff.

Now lets get into the interesting portion; Code Refactoring. Here we are going to move those conditional sections to a different procedure with all those logging stuff and not just that; the nested "If" conditions are also going to be flattened out. 
public bool CheckCondition()
{
    bool ReturnVal;

    var blnFlgA = ConditionA();
    var blnFlgB = blnFlgA && ConditionB();
    var blnFlgC = blnFlgB && ConditionC();
    var blnFlgD = blnFlgC && ConditionD();

    ReturnVal = blnFlgD;

    if (blnFlgA)
        Log("ConditionA success");
    else
    { Log("ConditionA Failed"); return ReturnVal; }

    if (blnFlgB)
        Log("ConditionB success");
    else
    { Log("ConditionB Failed"); return ReturnVal; }

    if (blnFlgC)
        Log("ConditionC success");
    else
    { Log("ConditionC Failed"); return ReturnVal; }

    if (blnFlgD)
        Log("ConditionD success");
    else
    { Log("ConditionD Failed"); return ReturnVal; }

    return ReturnVal;
}

//Calling the "CheckCondition" function
if (CheckCondition())
{
   ExecOps();
}
Here in the above code snippet, each function calls return value is stored onto individual flag variable. After which each flag variable is "ANDed" to the next Conditional functions, so as to prevent next function being called if the previous one failed. Finally all those logging stuff are handled based on the individual flag variables. Thats It and now we have trimmed and elegant version of the code.

No comments: