Pergunta

What steps and measures can I take to prevent deep indentations in my code?

Foi útil?

Solução

Deep indentation is usually not a problem if every function/method in your program does one and only one thing. Occasionally, it might be necessary to nest conditionals a few levels deep, but I can honestly say I've only written deeply indented code a handful of times in 12+ years of coding.

Outras dicas

The best thing you can do is extract methods:

int Step1(int state)
{
    if (state == 100)
    {
        return Step2(state);
    }
    else
    {
        return Step3(state);
    }
}

int Step2(int state)
{
    if (state != 100)
    {
        throw new InvalidStateException(2, state);
    }

    // ....
}

Maybe you could consider guard clauses?

instead of

public void DoSomething(int value){
    if (someCondition){
           if(someOtherCondition){
                if(yetAnotherCondition){
                       //Finally execute some code
                }
           }
    }
} 

Do

public void DoSomething(int value){
    if(!(someCondition && someOtherCondition && yetAnotherCondition)){
        return;
        //Maybe throw exception if all preconditions must be true
    }
    //All preconditions are safe execute code
}

If you ever get a chance I'd reccommend you read Code Complete by Steve McConnell. He's got a lot of great advise on these topics.

http://www.amazon.com/Code-Complete-Practical-Handbook-Construction/dp/0735619670/ref=pd_sim_b_6

For more about "guard clauses" see: https://sourcemaking.com/refactoring/replace-nested-conditional-with-guard-clauses

Invert your ifs.

Instead of:

if (foo != null)
{
    something;
    something;
    if (x)
    {        
       something;
    }
    something;
}
else
{
    boohoo;
}

I'd write:

if (foo == null)
{
    boohoo;
    return;
}
something;
something;
if (x)
{        
   something;
}
something;

The same applies to if-else blocks. If else is shorter / less nested, then revert them.

Check parameters' values in one place

Check all the parameters for illegal values as soon as you enter your method, then proceed knowing that you're safe. It makes for more readable code, but it also saves you piling up conditional blocks later on and spreading these checks all over the subroutine.

Typically, I have seen that deeply indented code is usually problematic code. If you are facing this problem, then step back and evaluate if your function is doing too many things.

At the same time, to answer your question, if there is a need for indentation that deep, I would suggest that you let it be there. For the simple reason that in such code, the indentation will help since it is likely to be a very long piece of code.

Break nested components (especially repeated ones) out into separate functions (this is easier if your language supports closures) or replace a series of nested loops with a recursion.

Also, indent two spaces instead of four.

I don't see deep indentations as a categorical problem to be removed (nor do I see refactoring as the true answer for everything).

Typically instead of nested ifs, I like to write logical statements:

if (foo && bar && baz) 

rather than

if foo 
 if bar
   if baz

I didn't believe it myself, but according to Code Complete this is an appropriate place to use break (if your team is on board). I'd imagine this is more acceptable with C++ programmers though, where break used in switch statements than it is with Delphi programmers where break is only used when you don't feel like writing a while loop.

Indentation is really a think to fight, indeed. What I learned to do is to divide the method into pieces first, then use a weird trick to skip every following pieces if one piece failed. Here is an example :

Instead of :

 {if (networkCardIsOn() == true)
     {if (PingToServer() == true)
        {if (AccesLogin(login,pass) == true)
             {if (nextCondition == true)
                ...
         }
     }
 }

I currently write :

 {vbContinue = true;

 if (vbContinue) {
       vbContinue = networkCardIsOn();
       if (vbContinue == false) {
             code to Handle This Error();
       } 
 }

 if (vbContinue) {
       vbContinue = PingToServer();
       if (vbContinue == false) {
             code to HandleThisError2();
       } 
 }

 if (vbContinue) {
       vbContinue = AccesLogin(login,pass);
      if (vbContinue == false) {
             HandleThisErrorToo();
       } 
 }
 ...

This has seem strange to me at a first, but since I use this, the maintenance cost has been divided by half, and my brain is cooler at the end of the day.

In fact, the gain introduced by this "technique" is that the code complexity is really divided because the code is less dense.

While reading the code, you don't have to remember anything about the past conditions : if your are at that point X in the code, the previous steps are passed and have succeeded.

Another gain is that "escape path and condition" from all those nested "if-else" is simplified.

Licenciado em: CC-BY-SA com atribuição
scroll top