|
Great Ghu NO!
1st style is fine. Everyone understands the else keyword.
2nd style is absolutely ripe for unintended bugs. You change the expression in the if () and then forget to make the corresponding change in the else if().
In both cases checking "== true" is a code smell (you don't understand logical expressions) and redundant. Checking "== false" is misleading and an easy source of errors.
Software Zen: delete this;
|
|
|
|
|
re/ ==true/false To finally set the record straight I do not write that way. I write if(expression) and if(!expression). I am not certain why I stuck the == true/false in the specimen. Perhaps to save readers the trouble of looking for the negation ! operator.
re/ 1st 2nd styles sometimes I favor the self documenting of the 2nd style for lengthy complex conditions except for the unintended consequence of compiler warnings stating not all paths return a value. So for that reason alone I will probably insist on 1st style.
re/ risking unintended bugs I am not worried as I am confident of being sufficiently careful also I do not expect the relevant code to require re-write. Though this is moot per above.
|
|
|
|
|
I prefer the K&R format and always use braces to promote code safety.
if (condition) {
...;
}
else {
...;
}/ravi
|
|
|
|
|
I make a sharp distinction between how I personally would like to format the code, and how I code when someone else will/might read it. I see so many errors caused by deep nesting and misinterpretation of indentation levels that I want a double check to detect mistakes. Therefore:
1) No indentation that is not supported by braces
2) No braces that do not adhere to the indentation level.
So if I see anything like
if (somecondtion)
xval = 1;
nextstatement; I immediately ask for permission to add the "redundant" braces. Most programmers accept this.
The second point is far less accepted: As long as you are not past the closing brace, you are still in the indented block. The brace is what causes the undent. Code is read in forward direction, so the last character position before the brace is still indented. Also, the closing brace visually blurs what is the next statement. I would really want to edit the above code to
if (somecondition) {
xval = 1;
}
nextstatement;- but few other programmers agree to the indentation of the closing brace. I have to tolerate what I consider a logical inconsistency, in non-private code. I also consider as inconsistent the style suggested by some:
if (somecondition)
{
xval = 1;
}
nextstatement;- you don't know that an indented block follows until after the opening brace. Inserting indenting space before it cannot be logically justified.
I honestly abhor deep block nesting, in particular when it ends up in a sequence of '} } } } }'. I try to avoid that whenever possible. Get out of a given nesting level as soon as possible! So I code like
FailureCode SomeMethod(int argX, float argY, sometype Z) {
if (argX > maxXvalue) {
return ErrorIllegalXArgument;
}
if (argZ == null) {
return ErrorUnspecifiedZ;
}
statement;
statement;
return Success;
}Today, this is accepted by most other programmers, at the method level, although the majority would like to add 'else's, including for the successful part. Sequences of 'else if' are by most not considered a new nesting level and usually accepted as a series of alternatives, although they strictly are not, by the grammar definition, of most C class languages.
A similar situation can occur in the middle of a method body: Several conditions must be satisfied for some operation to be performed. The C class of languages doesn't allow you to define arbitrary blocks (and certainly not to name them for the purpose of skipping out), so we have to 'abuse' e.g. a do statement:
do {
if (argX > MaxXvalue) {
break;
}
if (argY < 0.0) {
break;
}
if (argZ == null {
break;
}
statement;
statement;
}
while (false);I really would like to drop the 'while (false)'; (and preferably the introducing 'do' as well), but the compiler won't let me.
The 'classical' style, argued by some who consider themselves structural purists; insist that there shall be a single exit from every method; returning as soon as you discover a fatal error is messy coding, and the proper layout is like:
do {
if (argX <= MaxXvalue) {
if (argY >= 0.0) {
if (argZ != null) {
statement;
statement;
}
}
}
while (false);What really upsets people is if I want to get rid of the two first indentations as well, formatting it like
do {
if (argX > MaxXvalue) break;
if (argY < 0) break;
if (argZ == null) break;
statement;
statement;
}
while (false);It is my honest opinion that this code is far more readable than the fully structured purist code. I know of programmers who 'admit' that they agree about the readability. Yet, they will not accept this coding style; it is not 'proper'. So I reserve it for my private code that will never be handled by others.
Religious freedom is the freedom to say that two plus two make five.
|
|
|
|
|
I was recently influenced by an article recommending nesting be minimized. There are many such articles also here on this site. I also learned to detect and return from error cases as soon as possible if not immediately.
re/ braces and indentation etc. it is my opinion we all process visual information in our own way. My own main rule is to write / format in a manner which is pleasing to the eye exempli gratia as two specimens (not snippets as they were obviously not taken out from a code base) below.
re/ 1st specimen I prefer as below as I find your preferred style requiring greater cognitive load as there is simply more stuff to process but as stated we all process differently. Also there is the matter of screen real estate.
I find it helpful for braces to align. Also the IDE provides a vertical line of dots connecting same which I find quite helpful even rely on.
if (condtion) statement;
next_statement;
if(condition)
{
. statement;
. statement;
. statement;
}I do not understand the 2nd discussion. Why not merely write
if (!(argX > MaxXvalue|| argY < 0.0 || argZ == null))
{
statement;
statement;
}I agree w/ your final discussion. The purist style is a mess.
In closing I have never seen do {} while (false) I now realize it is identical to if(true) {}. Do you find this style more logical or pleasing to the eye.
modified 27-Jan-24 16:41pm.
|
|
|
|
|
BernardIE5317 wrote: I find it helpful for braces to align.
It provides a 'graphic consistency', but it breaks with the logic of braces and indents representing the same structural property. For the closing brace, you are not yet out of the block, yet you undent it.
Also, the first line, with the 'if' terminates abruptly without explanation. Putting the brace at the end of the line explains what is happening: You will now see an indented block that is the 'if' clause. As I illustrated, I accept a single short statement after the if-condition; then you will see a line terminated with a semicolon, and you know that the if-clause is complete, and no indented if-clause follows.
Another reason why I do not want the opening brace on a new line: It visually clutters the 'if'. With the 'if' having blank space underneath it, it stands out as the kind of statement owning the block; you can see from a far distance that is isn't a e.g. while or for loop. Similar for the closing brace: It clutters up what will happen after the conditional block.
I do not understand the 2nd discussion. Consider it a somewhat stylized, simplified example.
First, each of the tests may be far more complex than a simple comparison, and the total combined expression may be lengthy, maybe not fitting on a line.
More important: If the tests are conceptually independent, testing three separate aspects, that should be reflected by writing them as three separate tests. They are three different reasons for not performing the operation (or for returning from the method), not a single complex reason. A test such as 'if (ch1 >= 'a' && ch1 <= 'z')' is one conceptually homogenous test that belongs in one single if-condition. Three different tests on three distinct variables is rarely similarly homogenous.
Third: In many cases, a test that leads to the operation not being performed may lead to e.g. a status value being set. Each test may lead to a different status value. If you mold together the three tests, you cannot fit such handling into your layout rules.
Also, note that in my final example, which is my preferred one, the three conditions are clearly separated from the operation code, so that you see what is the condition, what is the operation. (I should have inserted a blank line after the conditions in the example with the 'break;' at separate lines - I always do in my code, and leaving it out here is at the level of a typo  .)
Yet another reason for not combining several conditions in a single if: I also abhor excessive use of or need for parentheses. Having to mentally break up and group the pieces of a complex logical expression is not good for readability. In my last example, you can immediately see that there are three distinct conditions, you see which elements are included in each, and the 'break' is an explicit indication that this condition may cause an operation to be skipped - analogous to an early return from a method.
For the record: I have looked at the binary code generated by the compiler for complex expressions compared to a sequence of simpler conditions. I have never, with a modern high quality compiler, seen any case where I could 'help' the compiler to created better code by lumping together as much as possible into large complex expressions. Even in cases where the code may be slightly different, timing shows that the more readable source code compiles to code executing just as fast.
Religious freedom is the freedom to say that two plus two make five.
|
|
|
|
|
Thank you for your kind explanation re/ the 2nd discussion. It makes sense.
re/ graphical consistency It seems to me text and its arrangement is a kind of graphics. I find it helpful to view code in just such a graphical manner than to study / examine further as needed.
Also I kindly request your opinion of the IDE feature id est vertical dots connecting vertically aligned matching braces.
- kind regards
|
|
|
|
|
BernardIE5317 wrote: I kindly request your opinion of the IDE feature id est vertical dots connecting vertically aligned matching braces.
Visual Studio provides such vertical dots regardless of brace alignment. It doesn't go by the braces, but by the statement structure. There is, however, no indication of inconsistent indentation: If you undent a few lines in the middle of a block, there is no warning. You might notice that the last line before the undent is not a single closing brace (as it should be) but VS won't tell you explicitly.
I am generally positive to the dot columns, but personally, I see moderate need for them. One reason is that I tend to keep the number of indentation levels so low that I can follow it by the indentations alone. Second: The dot columns really says nothing that the indentation doesn't. For me, the indentation is far more significant.
VS provides dot columns even for namespaces, classes and methods. I wish those could be separately turned off: If you really need to follow a dot column up or down, they just add to the bundle, making it more difficult to follow a specific one when you have to scroll screenfuls. I very rarely need support to know where a namespace starts and ends: With very few exceptions, all code in a source file is in the same name space. Large class definitions may have a source file to themselves, and if there are multiple classes in a single file, each of them is headed by a comment block that is hard to overlook. I need no dot column to see where a class starts/ends.
If you put the text cursor (the I-beam) immediately to the left of an opening brace or to the left of a closing brace, the matching one is highlighted. If I cannot see it (e.g. because it is outside the window); I can hit ctrl-å (on my Norwegian keyboard; I don't know what is is on a US keyboard) to jump to the matching brace. Doing it with the shift key down, ctrl-Å, will highlight the entire block, everything between and including the two braces. This reduces the need for the dot columns as well.
One feature that I use less frequently, but I am happy that it is available: In the window left edge, for every line opening a structure feature - name space, class, method, if-statements and loops - there is a box with a minus. Click it, and anything but the first line is collapsed. The box changes to a +. Click it (or the "..." box at the end of the line), and the hidden lines reappear. Using this feature, you can collapse, say, the inner three or four nesting levels for working on the not-as-deeply-nested parts of a method (so that it will fit within your editor window).
Most of all: When I have done extensive editing - in particular with a lot of moving code around, when you can very easily end up copying an opening brace but not the closing one - I go to the end of the workspace, delete and reinsert the closing brace. On reinsert, VS will reformat the entire workspace code according to the layout rules I have specified as my preferences. Then I can trust the indentations to reflect the block nesting correctly.
Given all of this, I see little need for those dot columns, but in VS they are so discrete that I am not bothered with them. I tried right now to turn them off, and I actually think that the structure now is visually clearer: With clean white space to the left of blocks, the indentations stand out more clearly. I think I will leave it off, at least for a period of time.
I have seen co-workers (using other editors) having set up their editor to display columns of bright yellow dots the size of full stops, on a black background. These are typically the people who also display each keyword and each delimiter type in a different color; a screenful of source code looks like an art painter's palette after a full day's work at the canvas. I find the bright yellow dots (and the multitude of color spots everywhere) to be very disturbing. So I am not unconditionally praising the dot columns; they must be discrete, subdued.
Religious freedom is the freedom to say that two plus two make five.
|
|
|
|
|
Thank You Kindly . It can be instructive to learn how others use their tools. re/ undents in middle of a block I rely on the extension "Continuous Formatting". It takes care of such things for me in real time.
I just now noticed the vertical dots as you stated. Good to know.
- kind regards
|
|
|
|
|
You probably will not favor the code I just now wrote but I find it easy to understand and pleasing to the eye and to the minds' eye. It is all on one line in my editor.
if (equal_to(minuend, subtrahend))if (!_limits) return false; else return !(*_limits)(0);
|
|
|
|
|
May I please inquire the manner in which code safety is promoted via the presented K&R format. I assume it refers to the risk of programmer error making the assumption braces reduce the risk of further editing incorrectly. That seems reasonable. Than there are the matters of personal preference re/ visual processing also screen real estate. Perhaps in future the IDE will provide a feature I have contemplated id est coloring each block in a differing color.
btw the matching braces which are not vertically aligned in your presented specimen prevent the IDE from displaying vertical dots betwixt them. A feature I quite prefer.
-kind regards
|
|
|
|
|
The K&R format is unrelated to code safety. You get that by using braces.
Using (or not using) the K&R style is purely a matter of personal taste and convention. Code in Java and Javascript are almost always written in K&R form, but that's purely by convention.
/ravi
|
|
|
|
|
I would rather use 'Assert' in the 'else {}' part to document the code and catch any possible problem, for example after a bad code editing.
|
|
|
|
|
This is a good question! It happens in real life and doesn't get discussed much though!
I recommend that you do *not* put in the extra test, because...
> If there is any future maintenance involving a change to the condition the future programmer will need to review both conditions. Extra comprehension, extra code - more likely an error or misunderstanding.
> a comment would serve the same purpose - and even can be removed safely
> ? If you have both tests, then ask what's the "else" for?
My experience - I've seen this done in the past, and it just trips up future programmer and confuses the human reader.
[my edit: - Absolutely agree with Iacopo Vettori's comment - use 'Assert' for such cases]
|
|
|
|
|
Every place I've worked that used C/C++ in the past 35 years dictated this style (and it's my style of choice).
if ()
{
}
else if
{
}
else
{
}
|
|
|
|
|
As you wrote it, the 1st style is correct.
Meaning an expression evaluates to true or false. The else if injects a potential question when re-reading the code that another possibility exists.
If it were if(boolvar==true)..., there may be cases where the else if is appropriate.
|
|
|
|
|
For short, simple IF/ELSE statements, I do not use the ELSE IF (false) because it is unnecessary.
If there are a large number of statements inside the IF block, then I'll use
IF (<Boolean test>) {
...
}
else
{
...
}
This allows me to document the code with a simple comment since the IF test is too far away from the ELSE.
Bond
Keep all things as simple as possible, but no simpler. -said someone, somewhere
|
|
|
|
|
BernardIE5317 wrote: if(expression == true) {...}
I had forgotten another variance on mandating usage.
if(true == expression)
Some C++ coders insisted on that because, the possibility existed that one might code the following.
if(expression = true)
And that is valid in C++.
Myself C++ programmers not controlling the scope of the their memory allocations was always, and still is, much more of a significant problem.
|
|
|
|
|
jschell wrote: if(true == expression)
sometimes referred to as Yoda formatting.
|
|
|
|
|
In reading all of the responses, I'm quite surprised that the only alternative mentioned was the tertiary operator (?: ). While it's simple to respond 'context dependent' as I saw in some of the replies, I think maybe it would help to spell out the concerns as well as offer up more choices.
Let's start with the elephant in the room... readability and maintainability. They both go hand in hand to answer the question "what are you trying to do or say in the code?"
In addition, some of the choices can lead to both bugs and code creep.
Let's start with if/else as a potential solution to express a/the logical path to traverse. A couple of simple rules would help (these are my rules, some may be obvious, but some are what I call 'Where's the curly brace' religious preferences. Take everything I write here are my own style rather than engage in religiosity).
if (condition 1)
{
}
else if (condition 2)
{
}
else
{
}
I always include the curly braces to ensure that scope is honored i.e. what code is executed based on which condition is met.
Too often I've see this:
if (condition 1)
else if (condition 2)
else
The problem here is if another author comes in to add/maintain the code, and adds a second line of execution to one of the conditions. If that author forgets to now add the curly braces to enclose scope, bad things happen.
if (condition 1)
else if (condition 2)
else
In addition, the whole Nickleback induced hatred the tertiary operator hails back to a late 70's bug introduced in a version (or couple of versions) of the Whitesmiths C compiler, and has since become lore. If we are striving for readability, then:
if (condition) ? "do one thing" : "do the other thing";
If you are vehemently opposed to this, chances are you also avoid the null coalescing as well as other similar constructs:
char *s = null;
char *t = null;
...
t = s ? "some string" : "some default";
But really, is this the only way to proceed?
How about:
switch (operand)
{
case 1:
break;
case 2:
break;
case 3:
break;
default:
break;
}
Once your application starts to grow in complexity, then you start to contend with nesting:
if (condition 1)
{
if (condition 2)
{
}
else if (condition 3)
{
if (condition 4)
{
}
}
}
This quickly becomes a nightmare to test and maintain. Testing requires the exercising of all logical branches, which this format complicates.
It quickly becomes evident that a better abstraction and/or implementation would be better suited. In the case of states, where a change of state triggers an execution of code and a transition to a new state, then a state machine would be better suited here to replace the if/else scaffolding (beyond the scope of this response).
And so it goes...
BTW, there are valid cases where the second format form of if/else would be better suited. For the case of nullable types i.e. bool? var, sometimes explicitly spelling out the true and false cases makes for better reading and comprehension:
bool? state;
...
if (state == true)
{
}
else if (state == false)
{
}
else
{
}
There's more than one way to skin and if/else clause! 
|
|
|
|
|
Stacy Dudovitz wrote: If that author forgets to now add the curly braces to enclose scope, bad things happen.
So not only coding now to guess at the competence of future programmers but also presuming that comprehensive unit testing will not happen?
|
|
|
|
|
To the first question about competence, that may be a bit harsh. We are, after all, human beings, and as such, even the best of us make mistakes. To be clear, while some mistakes can be attributed to competency levels and/or experience, I've also seen errors due to time pressure from the decisions makers, long hours of debugging a nasty problem, or other factors which can contribute to bugs.
Hence, my suggestion/recommendation falls more in line with defensive coding rather than assuming that everyone that is not me is an idiot. Your mileage may vary on that point.
With regards to unit testing, even in shops where unit testing is mandated as part of the development process (again, until a time deadline or other external factors discourage the practice in light of external pressures), my experience has been that often unit testing is more to tick off a checkbox than to actually produce maintainable and reliable code. Writing proper unit test code is an artform unto itself, and is predicated on other practices such as one method should encompass exactly one function. (Lets not split hairs on the triumvirate of the types of testing i.e. unit, system and integration tests. In this case I am referring to all three under the single banner of 'unit testing'.)
Bottom line, whatever practice you or your shop choose to implement, it should be purposeful and consistent. My post here was to offer a more thorough response to the original question.
|
|
|
|
|
Stacy Dudovitz wrote: often unit testing is more to tick off a checkbox
That can be solved with required code reviews and required code coverage metrics.
|
|
|
|
|
Agree on the curly braces. There was someone who did a lot of research about which if/else brace pattern worked best for automated merging, but I can never find it.
|
|
|
|
|
Do not attempt the extra check if you are dealing with floating points.
You might really need the extra ELSE clause.
if (x > 1.0) {
}
else if (x <= 1.0) {
}
else {
// yes you can reach this
throw wtf;
}
What are possible values of x?
|
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
