5

My IDE (IntelliJ IDEA) is telling me that I have the option to remove the braces on this if statement:

if (objectIsOfTypeFoo) {
    if (objectOfTypeFooIsShared) {
        // do something with Object of type Foo knowing that it's shared...
    } else {
        // do something with Object of type Foo knowing that it's not shared...
    }
} else if (objectIsOfTypeBar) {
    ...
}

To become:

if (objectIsOfTypeFoo) if (objectOfTypeFooIsShared) {
    // do something with Object of type Foo knowing that it's shared...
} else {
    // do something with Object of type Foo knowing that it's not shared...
} else if (objectIsOfTypeBar) {
    ...
}

I understand how this makes sense, and it's tempting to lose the indent, but my concern is that readability may suffer. The latter looks cleaner, but is the space saved worth the potential confusion?

I assume the performance difference between the two is trivial, if at all.

And as a follow up question: Is there a limit to how many 'if (condition)'s you can fit in a single line, or rather at what point does it become too many?

Liam
  • 1,041
  • 2
  • 17
  • 31
  • 4
    The first example is more readable and is less likely to cause issues to due to missed braces - IMHO – MadProgrammer Jun 13 '13 at 00:05
  • 2
    The performance difference at run-time is nil, since code-gen for both versions is identical. The performance difference at compile time is the cost of lexing a couple additional braces. So nil. Oh, and ignore the IDE: it is offering (in my opinion) horrible advice. – dlev Jun 13 '13 at 00:07
  • I've never seen IntelliJ recommend such a thing. I'd check your style settings. – duffymo Jun 13 '13 at 00:10

4 Answers4

8

I'm voting for the way you already have.

I don't even use this:

if(foo)
   return bar;

I like this instead:

if(foo){
   return bar;
}

"programs must be written for people to read, and only incidentally for machines to execute"

jsedano
  • 4,088
  • 2
  • 20
  • 31
  • 2
    +1 - yes indeed, but for the style AND the perfect quote. Personally, I'd recommend that you accept this one. – duffymo Jun 13 '13 at 00:09
  • 1
    I disagree, i think when an if is easy like if(foo)throw exception, is not bad. – nachokk Jun 14 '13 at 23:18
4

Always use braces. One day, you will want to put a second statement in your if or else block, and then you will wish you had. However, are you really making instanceof checks? Can you rework your program to turn them into polymorphic object behavior instead?

Eric Jablow
  • 7,874
  • 2
  • 22
  • 29
  • 1
    Often when someone writes complex code like this, further thought can simplify it. Even if it isn't easy to have different versions of `Foo`, perhaps `Foo` can hold an object that knows about the sharing strategy or has different subclasses with the sharing strategy. Remember Pascal's letter: "I apologize that my letter was so long. I did not have the time to make it short." – Eric Jablow Jun 13 '13 at 00:15
  • Yeah I need to generify the class, but for the time being I just need to get what I've got working! – Liam Jun 13 '13 at 00:19
2

I'd prefer the first one. I think that bit with multiple ifs on a single line is unreadable.

Sorry, but I'm going to vote to close. It'll be a debate without an answer.

duffymo
  • 305,152
  • 44
  • 369
  • 561
1

It's recommended to always use braces , but is there a situation when no body use braces and it is better not using them cause is more readable

if(condition){

} else if (condition) {
    ...
}else if (condition3){

}

if you always use braces it would be like this .Probably i mistake in some place.

if(condition){

 } else{ 

       if (condition) {
             ...
       }else {

           if (condition3){

           }//end if
       }//end else
 }//end else

So i think that using always depends of readability, like above said programs must be written for people to read and only incidentally for machines to execute.

nachokk
  • 14,363
  • 4
  • 24
  • 53