The following post is going to be an advanced curly-braces discussion with no right or wrong answer, just more “matter of taste”. It is about whether to put “else” (and other keywords, such as “catch”, “finally”) on a new line or not.
Some may write
if (something) {
doIt();
} else {
dontDoIt();
}
I, however, prefer
if (something) {
doIt();
}
else {
dontDoIt();
}
That looks silly, maybe. But what about comments? Where do they go? This somehow looks wrong to me:
// This is the case when something happens and blah
// blah blah, and then, etc...
if (something) {
doIt();
} else {
// This happens only 10% of the time, and then you
// better think twice about not doing it
dontDoIt();
}
Isn’t the following much better?
// This is the case when something happens and blah
// blah blah, and then, etc...
if (something) {
doIt();
}
// This happens only 10% of the time, and then you
// better think twice about not doing it
else {
dontDoIt();
}
In the second case, I’m really documenting the “if” and the “else” case separately. I’m not documenting the call to “dontDoIt()”. This can go further:
// This is the case when something happens and blah
// blah blah, and then, etc...
if (something) {
doIt();
}
// Just in case
else if (somethingElse) {
doSomethingElse();
}
// This happens only 10% of the time, and then you
// better think twice about not doing it
else {
dontDoIt();
}
Or with try-catch-finally:
// Let's try doing some business
try {
doIt();
}
// IOExceptions don't really occur
catch (IOException ignore) {}
// SQLExceptions need to be propagated
catch (SQLException e) {
throw new RuntimeException(e);
}
// Clean up some resources
finally {
cleanup();
}
It looks tidy, doesn’t it? As opposed to this:
// Let's try doing some business
try {
doIt();
} catch (IOException ignore) {
// IOExceptions don't really occur
} catch (SQLException e) {
// SQLExceptions need to be propagated
throw new RuntimeException(e);
} finally {
// Clean up some resources
cleanup();
}
I’m curious to hear your thoughts…
References: if – else coding style best practices from our JCG partner Lukas Eder at the JAVA, SQL, AND JOOQ blog.
If you have to write comment to if-else statement then there is something wrong. It leads to spaghetti code and you should create new methods for each statement which describes themselves instead.
I agree, sometimes methods can “resolve” such problems. But sometimes, the if-else really needs explanation, because some condition is quite subtle to understand…
You can comment if-else to clarify why you do something else. I see no reason why you can’t comment ANY code. If it’s useful for the next guy taking over your work, it’s useful.
I prefer not to comment code at all. Code should comment itself without the need for extra explanation. When I see a comment inside a code block, I always think that code is not written well enough. I think a developer should strive for the clearness of code itself, not the way it looks with comments.
That’s true in 90% of the code. But you’ll always run into situations (10%), where there is no really really clean way to express what you have to do in code. And there are two main reasons for that: Lack of time to refactor, lack of simplicity in the problem / requirement itself. That’s where comments really help.
I agree that sometimes we may need to comment code as mentioned, but as you’ve said that is rare (and it should be). So to keep this kind of comments rare, I think it’s better the code look ugly with comments. In this way, every time when you look at the code, you will desire to refactor it, rather than thinking that code looks good as it is :)
I find not commenting a very bad habit. Depending on your language choice of course a lot is clear. And you should indeed write understandable code. But for example I comment every function to explain what it does or why it’s needed. Also, often I make “smart decisions” that another developer might not understand or mess up, so I comment to explain my motivations.
True. And depending on the amount of time that passed since that “smart decision”, I might mess up myself, again ;-)
Put your “smart decisions” in methods that have names that describe exactly what they do and you really don’t those comments. If another developer using your API might mess up if he doesn’t understand those “smart decisions” then those decisions apparently didn’t result in clean code. And I wouldn’t call those decisions smart in that case. Refactoring is the first thing I’d do instead of writing comments about all the tricky things one should know before being able to use the code.
Lack of comments is one thing and only one thing… bad style. Code is NOT self documenting because all it can tell you is what it does. A comment should not simply answer “what?”, it should answer, “why?”
I like your comparison of “what” vs “why”. I don’t know how on earth I could write code in a way that it communicates precisely “why” a stock exchange order of clearing type code 27 needs to be grouped with all other subsequent orders of type code 27 (if and only if they have a rounding lot below 0.01), before actually unloading them within a time-frame of at most 35 seconds (fictional example in a real-life application).
But of course, not everyone goes far beyond writing “Hello World” programs where comments are indeed unnecessary :-)
Whoever says comments are redundant or code should be self documenting must really be taking the advice of TDD/Agile coach’s words LITERALLY!!! In real world people do use if-else conditions a lot and it WORKS!!
LukasEder: I liked your tongue-in-cheek response :)
P.S. I know someone will come and bite me for my comments here :P For all those who do, I do know all those golden principles of OO and et al..
You kinda have a point. Code tells you *what* it does, *why* (also known as specifications) is told by the tests. There is a very very narrow reason for commenting your code, business specification is not one of them. Everything that has business value, must be in *live* documentation aka. unit or acceptance tests.
If you write the first comment also *inside* the block (be it an if- or a try-block) then everything is consistent again — no need for the additional linebreaks.
That of course, is a valid alternative. Funny, I had never thought of that! :-)
If you’d stop over-commenting your code and improve the names of your methods, you wouldn’t need these comments at all. If you want to communicate that you are cleaning up resources, don’t call your method ‘cleanup’ but call it ‘cleanupResources’ instead. Using comments is a symptom that your code fails to communite its intend.
On the curly braces discussion; it is better to eliminate them for if-then-else and loops. Make sure you only call one method in each branch or loop, and communite its intend with the name of that method.
I guess you get paid by the length of your stack trace ;-)
This goes out to me as well as I have spent countless hours formatting code:
Take the damn OCB pills in the morning and stop caring about the details.
+1 On the comments that states that it’s better not to comment! I would see something like this
if(itNeedsToBeDone())
doIt();
Thats it. Clean, simple, easy to test
I rest my case :)
Of course. Clean and simple cases make up 90%. For the remaining 10%, see also eaorak’s answer
Ok. Is it Java style? No. Follow conventions and you’ll be happy!
Can you give an example of if-else (with comments) in “Java style”? What about commented try-catch-finally following conventions? I’m not aware of any strict conventions, so I’m glad to have a pointer…
Better then tenary and please bracket..
A better argument for your style might be that it is easier to comment out blocks out code
I do it exactly like you do it.
I usually do not use else.
void methodWithDescribingName() {
if (!something) {
dontDoIt();
return;
}
doIt();
}
This is far from best practices for If-Else statement IMHO. This is similar to ‘best practices’ that we can see in fragile programming books of old and should be better called ‘worst ceremonies’ for If-Else statement! Why is this so? There are several well known reasons, I’ll only repeat them here: 1. Code is read much more then it is written. So it should be readable and telling. Bunch of comments around If-Else will not help with reading, it will most probably create a mess instead. Code is written in a language, and that language should tell the reason why… Read more »
How would you write this business logic down into code, such that you can live without comments?
A stock exchange order of clearing type code 27 needs to be grouped with all other subsequent orders of type code 27 (if and only if they have a rounding lot below 0.01), before actually unloading them within a time-frame of at most 35 seconds (fictional example in a real-life application).
I find that pretty tough. See also Ian Littlewood’s remark about the difference between code communicating “what” it does, as opposed to code communicating “why” it does it.
(If it is not javadoc) I prefer to write the comment where I would write the log-output.
In fact – I prefer log-output and for your last example it would be much better:
log.trace(“Do some business”);
–
log.error(“IOException should not happen – ignored”, ignore);
–
log.fatal(“Propagate SQLException”);
–
log.trace(“Clean up some resources”);
–
etc … Also, my experience is that log-output is usually better maintained by the next developer who works with the code-base, while comments are usually ignored and might after a while end up in very bad places.
That’s a nice way of putting it. I had never though of using trace-logging that way. Quite nice!
I agree with the author that commenting the code looks better with the extra line. I’ll consider it.
I think that commenting goes for the whole block since it is a full conditional unit. It will help you understand the problem to solve and not necessarily all of the probable scenarios.
On the If sample, for clarity I never use IF ELSE IF ELSE ELSE …… I use just IF ELSE one time and I put the comment on the top for both the If and the else if any. I prefer using CASE (switch) or several IFs. On the try sample I prefer the first one, this is because I like to follow an standard. If every statement has a left curly bracket at the end and you close that statement with a right curly bracket in a standalone line. In a Try I should follow the same logic ,… Read more »
i actually write my code the same way as you do. much easier to read imo.
How about this?
// This is the case when something happens and blah
// blah blah, and then, etc…
if (something) {
doIt();
// This happens only 10% of the time, and then you
// better think twice about not doing it} else {
dontDoIt();
}
I try to use your way where I work but they “correct” me before I can commit. More than once there’s been logic errors due to a nested if block and developers mistakenly think the following else belongs to the inner if, not the outer. That has nothing to do with naming of methods or comments, simply scanning with your eyes quickly the structure is unclear without else on a new line.
I can’t say one way or the other on the comments but I definitely prefer all control statements never have a brace in front of them… ie the ultimate 1TBS (one true brace style). I had to google to see if I was the only one that thought this. This is because the braces are pretty much white space. In fact I am pretty sure most programmers could read the code with out the braces (if you don’t believe me there is a whole language that does this … python). So having an “else” or “else if” indented by one… Read more »