views:

250

answers:

10

Say we have the following code

function ABC() {
    if (....) {
     if (....) {
      if (....) {
       ...
      }
     }
     else {
      ...
     }
    }
    else {
     ...
    }
}

my question is... Is that 3 levels of If/while-blocks already way too much to withstand?

if not, how much levels of these statement blocks can you tolerate?

will it be much more tolerable if the function only do simple things like

function ABC() {
    if (x>0) {
     if (y<0) {
      if (x*y>r) {
       r+=1;
      }
     }
     else {
      y+1;
     }
    }
    else {
     x+1;
    }
}

Thanks for reading, it will be great if you can leave some advice to me

+1  A: 

Anytime it starts to look like an arrow I would start to think about abstracting it.

I personally think it looks a lot better to add some returns as "guarding clauses" whenever possible.

if(t)
   return r+1;
if(v)
   return r+1;

Arrow Code

Arrow Anti-Pattern

cgreeno
A: 

Those three blocks arent so bad considering you'd need about 5 or 6 conventional ifs to carry out the same semantic meaning without doing nested ifs. However if you get more than 5 (to pick an arbitrary number ;) ) or so of complex if statements it might get a bit hard to read.

RCIX
+8  A: 

You should try the Linus Torvalds approach, whenever possible:

Instead of

if (something()) {
  do_this();
  if (something_else()) {
    do_that();
  }
} else {
  something_completely_different();
}

Try doing this:

if (!something()) {
  something_completely_different();
  return;
}

do_this();

if (something_else()) {
  do_that();
}

Basically, use a reversed if, include only the original else part in it, and return at the bottom.

This will not always apply, but when it does, it simplifies things a lot.

Other than that: refactor!

Daniel Schierbeck
A: 

My rule of thumb is that when it's too hard to tell by the inner if what conditions are needed to reach there, then it's too deep... Nothing concrete, but it comes down to what you can read...

Stobor
A: 

Its definitively to much nesting. Try to put the conditions into a single one ore use a switch statement. You can also use more then one return statement to clean up the code. Inverting the condition is another technique to reduce complexity in some situations.

function ABC(x, y, r) {
    if (x>0 && y<0 && x*y>r) return r+1;
    if (x>0 && y>=0) return y+1;
    return x+1;
}
Fu86
A: 

I find 3 levels of identation starting to be difficult to read. When, for some reason (:P), I'm using more if statements I invert the logic, doing so helps you avoiding right identation to near the center of the page, which I find to be my limit after which I start to get annoyed.

See this for more http://stackoverflow.com/questions/268132/invert-if-statement-to-reduce-nesting.

Alberto Zaccagni
A: 

In my opinion, 3 levels or above points at a design flaw.. You should probably rethink your algorithm a bit, since to many will make it kinda complex and hard to read.

MüllerDK
+5  A: 

In the spirit of Robert C. Martin: code should read like a story.

If you are having problems understanding what exactly happens in a function you should refactor it. There is no golden rule stating that 3 levels of nesting is the maximum. In some cases 3 levels could be ok, where in other cases it's way too much. With more nesting you will often have a methods do several things. This can make the method more complicated to understand.

Book tip: "Clean Code: A Handbook of Agile Software Craftsmanship" by Robert C. Martin

elmuerte
A: 

I'd rather write it sequentially:

function ABC() {
    if (x <= 0) {
        x+1;
    } else if (y >= 0) {
        y+1;
    else if (x*y>r) {
        r+=1;
    } (else... not given in example)
}
G B
+1  A: 

Also, I think it's invaluable to extract your condition checks into their own functions. That way they can read like a comment and are much easier to understand.

if (someComplicatedCondition()) {
  goAndDoSomeRealCoolThing();
}
else {
  goAndDoSomethingElse();
}

If at any stage you feel you need to add a comment amongst your if/when's etc.. chances are you need some method extractions.

gommo