What is the problem with excessive indentations in Java if-else statements?

[ArthurRead]

Hello everyone. I am a little confused as to how I should organize if else statements in java. Here is an example from my Leap Year program:

 if (x<1600) {
         System.out.println(setError);
      } else if (x>2013) {
         System.out.println(setError);
      } else {
         if (c == 0) {
            System.out.println(leap);
         } else if (b == 0) {
             System.out.println(notLeap);
         } else if (a == 0) {
             System.out.println(leap);
         } else {
             System.out.println(notLeap);
      }
      }

Is this fine? Is it against convention? Can you guys follow it easily?

Thank you

 

[rcgldr]

Shouldn't the next to last right brace be indented?

 

[jtbell]

Purely as far as indentation is concerned, I would do it like this:

if (x<1600) {
    System.out.println(setError);
} else if (x>2013) {
    System.out.println(setError);
} else {
    if (c == 0) {
        System.out.println(leap);
    } else if (b == 0) {
        System.out.println(notLeap);
    } else if (a == 0) {
        System.out.println(leap);
    } else {
        System.out.println(notLeap);
    }
}

Everything that is at the same "logical level" is aligned vertically.

 

[Cod]

Here's how I'd indent the block of code...

if (x<1600) {
    System.out.println(setError);
} 
else if (x>2013) {
    System.out.println(setError);
} 
else {
    if (c == 0) {
        System.out.println(leap);
    } 
    else if (b == 0) {
        System.out.println(notLeap);
    } 
    else if (a == 0) {
        System.out.println(leap);
    } 
    else {
        System.out.println(notLeap);
    }
}

The instructors I've learned from in the past don't like ending braces being on the same line as starting braces. This method also makes it easier to read code in my opinion.

 

[D H]

jtbell and Cod used the same style, a variant of the K&R style sometimes called Stroustrup style, after the inventor of C++. A related variant is 1TBS, the "one true brace style":

if (x<1600) {
    System.out.println(setError);
} else if (x>2013) {
    System.out.println(setError);
} ...

Here the close brace and else are on the same line. The rationale is that one shouldn't separate the close brace and the else with a newline as it is illegal to put random code between the close brace and else. A "true brace" is one before or after which one can insert arbitrary statements.


A different style is Allman. Here the open brace gets a line of its own:

if (x<1600)
{
    System.out.println(setError);
}

else if (x>2013)
{
    System.out.println(setError);
} ...

There are lots of others. Which one is right? That's getting into a religious war. Studies abound regarding readability, maintainability, etc., and there's no clear winner.

What is clear / nearly universally agreed upon:

• Eight space indentation, as is done in the opening post, is too much indentation.
• Omitting the braces because the code block is a single statement is an open invitation to Murphy law. Always use braces.
• Be consistent. Don't switch from 1TBS to Allman to GNU style within a function. Or within a file, or within a set of related files. How far should one pursue this consistency? Make it project-wide and religious wars will start to break out amongst developers.
• Be adaptive. If you have to change one line in a 500 line file, and that file is written in a style you feel is atrocious, suck it up. Do not change the entire file to suit your personal preferences.
• Be flexible. Eventually you'll work on a project that has mandated some goofy style. BTW, it will inevitably be a bad one because a good project manager knows not to do that. You'll either have to live with it, fight it, or find another job.

 

[jtbell]

Now that we've gotten into placement of curly braces, I use Allman style myself. (That's why I said "purely as far as indentation is concerned".) For me, it makes it easier to see if there are any missing, extra, or wrongly nested curly braces.

I agree with D H's list of points.

Veering into a different topic, I prefer to use the "else if" construction only when making a series of comparisons on the same variable, as in the first part of ArthurRead's example; not when comparing different variables, as in the second part. I would have written the code this way, using Allman style:

if (x<1600)
{
    System.out.println(setError);
}
else if (x>2013)
{
    System.out.println(setError);
} 
else
{
    if (c == 0) 
    {
        System.out.println(leap);
    }
    else 
    {
        if (b == 0)
        {
            System.out.println(notLeap);
        }
        else
        {
            if (a == 0)
            {
                System.out.println(leap);
            }
            else
            {
                System.out.println(notLeap);
            }
        }
    }
}

However, any time I find myself writing such a deeply nested set of if-statements, I stop, review the logic carefully, and try to find a simpler way to do it.

 

[Grep]

Yep, D H is quite right with his points.

I'll just add that the vast majority of Java programmers and code use a standard Java coding convention. I really suggest you read it and get to know it well. For example, the section dealing with if/else/else-if: http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html#449

I would also space out your expression with spaces around the operators. For example:

    if (x < 1600) {
        ... etc ...

This is in the part of the coding standard that deals with whitespace.

 

[D H]

I'll just add that the vast majority of Java programmers and code use a standard Java coding convention. I really suggest you read it and get to know it well.

That's one of the many things I don't like about Java. I switched from 1TBS a long time ago. How do you comment an else block?

 

[MrTheBard]

I agree with DH. I didn't know that the style was called 'Allman', but that's definitely an organized way to write your code.

if(condition = condition)
{
result
}

Having the braces on their own lines makes it easy to identify exactly what is enclosed in the else/if statement.

 

[Borg]

jtbell and Cod used the same style, a variant of the K&R style sometimes called Stroustrup style, after the inventor of C++. A related variant is 1TBS, the "one true brace style":

if (x<1600) {
    System.out.println(setError);
} else if (x>2013) {
    System.out.println(setError);
} ...

Here the close brace and else are on the same line. The rationale is that one shouldn't separate the close brace and the else with a newline as it is illegal to put random code between the close brace and else. A "true brace" is one before or after which one can insert arbitrary statements.

I prefer jtbell's and Cod's style as well but, most of the developers on my project use Allman.

I can live with that but there is one coding style on the project that I would kill if it was in my power to do so. Several developers on the project have their Eclipse IDE set up to limit the width of coding lines to 80 characters. When you get deep enough into some loops and if/else statements (i.e. lead spacing of 30 or more), the code turns into unreadable junk piled up on the right because their Eclipse autowraps at or before 80 characters whenever they save the file. I think that I know how and why it happened and even who started it years ago but, getting people to change can be difficult. Fortunately, most of the code isn't like this.

What is clear / nearly universally agreed upon:

• Eight space indentation, as is done in the opening post, is too much indentation.
• Omitting the braces because the code block is a single statement is an open invitation to Murphy law. Always use braces.
• Be consistent. Don't switch from 1TBS to Allman to GNU style within a function. Or within a file, or within a set of related files. How far should one pursue this consistency? Make it project-wide and religious wars will start to break out amongst developers.
• Be adaptive. If you have to change one line in a 500 line file, and that file is written in a style you feel is atrocious, suck it up. Do not change the entire file to suit your personal preferences.
• Be flexible. Eventually you'll work on a project that has mandated some goofy style. BTW, it will inevitably be a bad one because a good project manager knows not to do that. You'll either have to live with it, fight it, or find another job.

Speaking of inconsistancy. We had a temp who coded all of his class level variables like this:

	public 
	static
	final
	long 
	myVariable
		= 0;

One weekend he even decided to come in and change a huge number of class files in the repository to that format. He didn't last very long.

 

[D H]

I can live with that but there is one coding style on the project that I would kill if it was in my power to do so. Several developers on the project have their Eclipse IDE set up to limit the width of coding lines to 80 characters.

I'm a firm believer in the 80 character limit.

Well, almost. 80 characters is on the verge of being too long. Python, for example, has a 79 character limit.

Some say the rule exists only because of old-style technology. They're right, but it's not the technology they are thinking of that is the basis for this rule. It's very old style technology: It's about the human eye and brain. The eye has to move and reacquire the line at the transition from the end of one line to the start of the next. This transition is not jolting if it's done autonomously, and that means short lines. Reading comprehension drops when the brain has to intervene to help the eye find the start of the next line.

When you get deep enough into some loops and if/else statements (i.e. lead spacing of 30 or more), the code turns into unreadable junk piled up on the right because their Eclipse autowraps at or before 80 characters whenever they save the file.

"Doctor! It hurts when I do this!" ⟨Bonk⟩

So don't do that, then.

People have occasionally tried to show me sample code to demonstrate why I should lift the 80 character limit that I impose whenever I have say on coding standards. The code almost inevitably falls into one (or both) of the categories shown below. The problem isn't the 80 character limit. The problem is the code itself.

Code {
   that {
      looks {
         like {
            this {
               has {
                  a {
                     name: {
                        // The arrowhead antipattern.
                     }
                  }
               }
            }
         }
      }
   }
}code.that.looks.like.this.has.a.name.too (it.violates.the.law.of.demeter);

In both cases the nesting is way too deep. Deep structural nesting (nested ifs, loops, etc.) and deep access nesting (foo.bar.baz.qux) should be viewed as a sign of something wrong with the code rather than something wrong with the 80 character limit. The 80 character limit acts to highlight the problems, and that's a good thing, not a bad thing.

 

[Borg]

I understand that there shouldn't be really long lines. The problem that I mainly have is letting the IDE perform the breaks for you. It doesn't even have to get that deep.

The current code that I work on has been worked on for more than 10 years with lots of developers of varying skills coming and going. I have seen if statements that approach 200-300 characters before hitting the first bracket so that you get a mess like this when the IDE sets the breakpoints:

	private void someRandomMethod(String sql) throws SomeException{
		try {
			if (someBoolean) {
				ResultSet resultSet = null;
				Connection connection = null;
				Statement statement;
				try {
					connection = getConnection();
					statement = connection.createStatement();
					boolean success = statement.execute(sql);
					if (success) {
						resultSet = statement.getResultSet();
					}
					else {
						throw new Exception("Statement failed to execute.");
					}
					ResultSetMetaData rsmd = resultSet.getMetaData();
					int columnCount = rsmd.getColumnCount();

					while (resultSet.next()) {
						String data = String.valueOf(resultSet.getString(
                                                    "SOME_LONG_TABLE_NAME"));
						[B]if ((isOneTypeOfData || 
							hasPassedChecksInThisLongMethod(firstVar, 
								secondVar, "Some random text", data)) && data
							!= null && (checkWhatThisMethodSays() || 
							checkThisIfTheFirstDidntPass(data)) && (anotherBool
							&& todayIsNotTheFirstOfTheMonth)) {
						}[/B]
						else {
							...
						}
					}
				}
				catch (Exception e) {
					e.printStackTrace();
				}
				finally {
					...
				}
			}
			else {
				...
			}
		}
		catch (Exception e) {
			e.printStackTrace();
		}
	}

Good luck trying to understand the if statement after the resultSet.next(). It follows the break at or before 80 rule but is completely undecipherable. The depth of the indentations isn't terribly unreasonable for what's being done but the long names and method calls easily push the code over the 80 char limit. The original if statement may not have even started out being too long but grew as various changes or fixes were implemented. Still, the ridiculously long if statement could be readable if the IDE wasn't choosing where to insert breaks.

 

[Borek]

+1 on Allman.

 

[Vanadium 50]

I personally like commenting the bracket that terminates the if clause, describing the if condition, e.g. "} // leap year". This protects one against code growing into a monster. Sometimes expedience requires living with bad design.

 

[D H]

The current code that I work on has been worked on for more than 10 years with lots of developers of varying skills coming and going. I have seen if statements that approach 200-300 characters before hitting the first bracket.

That code exemplifies the arrowhead antipattern. Google that phrase. 200 to 300 characters before hitting the first brace? That's not just the arrowhead antipattern. That is code gone wild, obscenely wild.

One of your problems is that eight space indent. That has long been considered excessive. Modern coding standards typically dictate between two to four. Another problem is excessive cyclomatic complexity. (I prefer extended cyclomatic complexity over McCabe's original concept.) Set a limit on cyclomatic complexity. Some organizations set the limit as low as ten. Everything over that limit needs a waiver. It keeps people in line.

Do have a development process?

 

[Borg]

That code exemplifies the arrowhead antipattern. Google that phrase. 200 to 300 characters before hitting the first brace? That's not just the arrowhead antipattern. That is code gone wild, obscenely wild.

One of your problems is that eight space indent. That has long been considered excessive. Modern coding standards typically dictate between two to four. Another problem is excessive cyclomatic complexity. (I prefer extended cyclomatic complexity over McCabe's original concept.) Set a limit on cyclomatic complexity. Some organizations set the limit as low as ten. Everything over that limit needs a waiver. It keeps people in line.

Do have a development process?

It was just a made-up example to illustrate how the code can get mangled by an IDE. The tab indents were only 4 spaces on my IDE and must have been changed to 8 when I copied them into the thread. The depth of the bracketing is only five levels inside the method (the extra try/catch could easily have been a for loop).

Yes, I agree that the code is obscene in places but I can't do much about the millions of lines of code that already exist (a quick check shows over 80K files). I think that rather than derail the OP's original thread further, we should start another one. How do you deal with out-of-control code?