Refactoring: avoiding nested conditional statements

Recently at I was given the task of adding an new validation routine to an existing validation process.  In this piece of code, the requirements mandated that a series of sequential tests would be run, but in the event of a failure of any of them, the process would kick out and set an error state, provide user feedback, and whatever other tasks needed to occur.  We have all seen processes like this before.  Essentially it looked like this:

error = true;
if ( testOne() )  {
    if ( testTwo() ) {
        if ( testThree() ) {
            if (testFour() )  {
                 error = false;
                 doAllTestsPassedStuff()
            } 
        }
    }
}
if ( error ) {
	handleErrorCondition()
}

Looking at this block of code, the intent is pretty obvious as we progressively run tests as long as the previous test returned true, eventually firing the doAllTestsPassedStuff() method.  If any of the tests failed, we would call handleErrorCondition().  While this approach is completely functional, the maintainability of it is no fun, and it just feels wrong to me.  For the task I was given, I had to add a new test davesSuperTest()  between the 2nd and 3rd conditional blocks.   If I were to follow the previous approach, I would insert it there, and tab out the previous testThree() and testFour() conditional statements further to the right.  In my opinion this is an ugly block that is getting uglier by the minute.

By altering the approach to use try/catch blocks, we can still maintain the same level of control and order or operations as dictated in the requirements, but each condition becomes insulated from the others,  like this:

try {
	if ( !testOne() )	{
		throw "fail:testOne";
	}
	if ( !testTwo() )	{
		throw "fail:testTwo";
	}
	if ( !davesSuperTest() )	{
		throw "fail:davesSuperTest";
	}
	if ( !testThree() )	{
		throw "fail:testThree";
	}
	if ( !testFour() )	{
		throw "fail:testFour";
	}
	// if we reach this point, then all of the above tests passed.
	 doAllTestsPassedStuff()
}	
catch(e)	{
	handleErrorCondition()
}

Given this approach, it is very simple to add/remove conditions without disrupting other conditions, even better, I don’t have to scroll to my second monitor on the right!

4 thoughts on “Refactoring: avoiding nested conditional statements

  1. Isn’t try/catch slower then actually handling the error properly?

    We use try catch around things like a webservice call which we can’t control the result erroring or not.

  2. Paul, I would imagine that completely depend on the language being used, and the engine that is processing it. This topic wasn’t really directed to a specific language at all, but more about the approach in general. The example that prompted this blog post was actually Javascript. With regard to ACF though, which I imagine you are referring to, I believe that you are right, but I haven’t tested it in several versions now.

  3. Some languages, like Python or Java, create an object every time an exception is thrown. As long as you use exceptions for “exceptional” situations, (ie, don’t use it as regular control flow, esp. in a hot loop), you’re golden.

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>