Home » Avoid… » Mountains

Mountains

Overview

Nested logic occurs when if-then-else blocks, try-catch blocks, switch statements, etc are placed within each other. The whitespace on the left side of the resulting code, if rotated by 90° counterclockwise, looks like mountains, giving this article a name. Mountains height grows when the nesting gets deeper.

A possible cause for nested logic is that long ago developers were teached to have 1 return statement per function. It is a part of an outdated “Structural Programming” paradigm from the 1960-es. Today there is simply no need for a single exit point, unless you programming language has a restriction. In most languages you can have as many exit points from a method as you need, and in modern languages you can also leave method by throwing an exception.

Example

        bool IsSomething()
        {
            if (_thisFlag)
            {
                if (_thatFlag && !_anotherFlag)
                {
                    result = true;
                }
                else if(_anotherFlag)
                {
                    switch (_someEnum)
                    {
                        case ConsoleColor.Black:
                        case ConsoleColor.White:
                            result = true;
                            break;
                        case ConsoleColor.Red:
                            result = false;
                            break;
                        default:
                            if (IsTooDark(_someEnum))
                                if (_thatFlag)
                                    result = true;
                                else
                                    if (!IsTooDisturbing(_someEnum))
                                        result = true;
                            break;
                    }
                }
            }
            return result;
        }
  visualisation of the mountains

Many examples can be found online, for example – this is a nice one: Coding Like the Tour de France (“The Daily WTF”, 2008-01-28, by Alex Papadimoulis).

Why to avoid

  • It is less readable than the flat code. With several blocks of code inside nested curly brackets you’ll need to know exactly which opening bracket belongs to which closing bracket and other way around. It is also hard to see when some logic will be applied or not. And a small misalignment can already lead to the wrong understanding of logic.
  • It is not clear what is a normal behaviour (the happy path) and what is not. The alternative implementation – using guard clauses instead of nested logic – will reveal the happy path with all its preconditions.
  • This is an anti-pattern called “Arrow”. Read more about it here. Here is a good example with explanation of how to refactor it and why: “Flattening Arrow Code”, Coding Horror.

Alternatives

Alternative is a combination of:

  • Guard Clauses to leave the method immediately if any precondition is not satisfied. This is called “early exit”.
  • Extracting methods with chunks of logic happening in different logical paths.

Coding

Here is an example of nested logic and how it can be refactored, in pseudo code.

SomeMethod(argumentUsedInSomeConditions)
{
	if (condition1)
	{
		switch (enumValue)
		{
			case enumA.SomeValue:
				if (condition2)
				{
					switch (string)
					{
						case "some special string":
							//do some logic #1
							...
							...
							//do some logic #2
							...
							...
							if (condition4)
							{
								if (condition5)
								{
									//do some logic #3
									...
									...
									if (condition6)
									{
									   //do some logic #4
									   ...
									   ...
									}
								}
							}
							break
						case "not a special string":
						case "another not special string":
						case "and one more string":
							//do some logic #5
							...
							...
							break
					}
				}
				break
		}
	}
}

All together it was almost 100 lines of code. In my experience, both the complexity and the pain caused by working with nested logic in the code increases with the number of lines wrapped into the outer if-condition.
Actually, I saw much more excessive examples in real code, with nesting level of almost 20, so you had to scroll horizontally to see the code completely!

Refactoring was done in two steps:

  • First each branch of nested logic is extracted to a dedicated method (or to several methods if required). Each method is called according to what it is supposed to do. After this is done, the parent method ‘SomeMethod’ becomes a way smaller and provides a higher level overview on what is going on. The method did different things based on different preconditions before. Now the logic to do different things is separated from the logic checking the preconditions, this is another advantage – a better separation of concerns.

    SomeMethod(argumentUsedInSomeConditions)
    {
    	if (condition1)
    	{
    		switch (enumValue)
    		{
    			case enumA.SomeValue:
    				if (condition2)
    				{
    					switch (string)
    					{
    						case "some special string":
    							DoSomething1
    							DoSomething2
    							if (condition4)
    							{
    								if (condition5)
    								{
    									DoSomething3
    									if (condition6)
    									{
    									   DoSomething4
    									}
    								}
    							}
    							break
    						case "not a special string":
    						case "another not special string":
    						case "and one more string":
    							DoSomething5
    							break
    					}
    				}
    				break
    		}
    	}
    }
    


    The biggest branch of logic is the ‘case “some special string”‘ part. In order to keep the parent method short and in order to reduce the level of nesting, I decided to extract this code into a dedicated method ‘DoSomethingSpecial’:

    private DoSomethingSpecial
    {
    	DoSomething1
    	DoSomething2
    	if (condition4)
    	{
    		if (condition5)
    		{
    			DoSomething3
    			if (condition6)
    			{
    				DoSomething4
    			}
    		}
    	}
    }
    

  • Given, previous step is completed, the Guard Clauses can be introduced. This is a main trick to get rid of the mountains – leave every method immediately if any precondition is not satisfied. This is called “early exit”. Note for .NET developers: ReSharper detects candidates for an early exit automatically and suggests to auto-invert if conditions to reduce the nesting. The result is a flattened structure with no nesting at all:

    SomeMethodRefactored(argumentUsedInSomeConditions)
    {
    	if (!condition1 || enumValue != enumA.SomeValue || !condition2)
    		return
    
    	if (string == "some special string")
    	{
    		DoSomethingSpecial
    		return
    	}
    	
    	OtherSupportedStrings = Array { "not a special string",
    					"another not special string",
    					"and one more string" }
    	if (!OtherSupportedStrings.Contains(string)
    		return;
    	
    	DoSomething5
    }
    
    private DoSomethingSpecial
    {
    	DoSomething1
    	DoSomething2
    	
    	if (!condition4 || !condition5)
    		return
    	
    	DoSomething3
    
    	if (!condition6)
    		return;
       
       DoSomething4
    }
    

Final side-by-side comparison

Mountains, nested logic:

SomeMethod(argumentUsedInSomeConditions)
{
	if (condition1)
	{
		switch (enumValue)
		{
			case enumA.SomeValue:
				if (condition2)
				{
					switch (string)
					{
						case "some special string":
							//do some logic #1
							...
							...
							//do some logic #2
							...
							...
							if (condition4)
							{
								if (condition5)
								{
									//do some logic #3
									...
									...
									if (condition6)
									{
									   //do some logic #4
									   ...
									   ...
									}
								}
							}
							break
						case "not a special string":
						case "another not special string":
						case "and one more string":
							//do some logic #5
							...
							...
							break
					}
				}
				break
		}
	}
}
  Flat code with no nesting:

SomeMethodRefactored(argumentUsedInSomeConditions)
{
	if (!condition1 || enumValue != enumA.SomeValue || !condition2)
		return

	if (string == "some special string")
	{
		DoSomethingSpecial
		return
	}
	
	OtherSupportedStrings = Array { "not a special string",
					"another not special string",
					"and one more string" }
	if (!OtherSupportedStrings.Contains(string)
		return;
	
	DoSomething5
}

private DoSomethingSpecial
{
	DoSomething1
	DoSomething2
	
	if (!condition4 || !condition5)
		return
	
	DoSomething3

	if (!condition6)
		return;
   
   DoSomething4
}

Guard Clauses & further reading

Guard clauses are exactly these if-conditions, followed by return statement. They are like guards, guarding the happy path. If the conditions are not matching, they will ask to leave before the happy path code is reached. This topic is well covered, just google for “Guard Clauses” or read a chapter ‘Replace Nested Conditional with Guard Clauses’ in the “Refactoring” book by Martin Fowler.

I also find that this “Maintainable Code” presentation has a good coverage of guard clauses, mountains, and a bunch of other good practices (link from Andy Palmer’s blog).

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: