views:

922

answers:

21

So I know it's considered somewhat good practice to always include curly braces for if, for, etc even though they're optional if there is only one following statement, for the reason that it's easier to accidentally do something like:

if(something == true)
    DoSomething();
    DoSomethingElse();

when quickly editing code if you don't put the braces.

What about something like this though:

if(something == true)
{   DoSomething(); }

That way you still take up fewer lines (which IMO increases readability) but still make it unlikely to accidentally make the mistake from above?

I ask because I don't believe I've ever seen this style before for if or loops, but I do see it used for getter and setter in C# properties like:

public string Name 
    {get;set;}

Not asking what's best since that's too subjective, but rather just whether this would be considered acceptable style and if not, why not.

+9  A: 

Instead of:

if(something == true)
{   DoSomething(); }

Do this:

if(something == true) {   DoSomething(); }
Nicholas Mancuso
No don't do that. I know this is a C# question and I know the Visual Studio debugger steps over that as two separate statements, but in other languages or debuggers when you step over that one line its not always clear whether the 'then' clause was executed or not. It can be very ambiguous.
Brian Ensink
I'd rate up the comment if I could, should put it as an answer
Davy8
+19  A: 

When I come across a one-line if statement, I usually skip the curlys and keep everything on the same line:

if (something == true) DoSomething();

It's quick, easy, and saves space.

Stephen
I just can't get on with that style. It means that if I see an 'if', I've now got two places to look to see what it does.
Roger Lipscombe
A: 

There's no problem there... in fact, Visual Studio won't put that code on it's own line if you try to auto-format it. So, if that's more readable to you... you're good.

Timothy Khouri
+3  A: 

That way you still take up fewer lines (which IMO increases readability)

I disagree that having fewer line breaks increases readability. The layout of the code should make its structure more visible, not hide it.

florin
Florin, perhaps you could edit your response to add spaces between the words?
DOK
I agree with DOK. He has spaces, and he didn't say leave out spaces, just have fewer lines.
Aaron Smith
soooo... According to him then we should ... hmmm... write every word on a different line ... Is that the point ?
Newtopian
My original message had no spaces between the words, to make a point that reducing the amount of syntax does not necessarily improve the comprehensibility of the code. The formatting was intentional, so I disagree with DOK and Shy.
florin
A: 

If you really want to save lines of code, you can type it like this:

if(something == true) { DoSomething(); }

or this, without the braces

if(something == true) DoSomething();
DOK
+6  A: 

If you work in a team, you need come up with a standard.

Personally I like doing:

if(foo)
    DoSomething();

or

if(foo) DoSomething();

I don't see a problem with not having the braces. The reason people cite, the one you mention about adding another statement on the line below, is one that I've never run in to.

jonnii
I've run into it many times. You want to put a debug print saying that at this point foo was true, and you end up printing foo if it's true, but doing DoSomething regardless of the value of foo.
Nathan Fellman
+3  A: 

I don't see why not. I've also used it for short functions.

On the offensive style front, it's far better than the unspeakable:

  if (something== true)   {
      DoSomething();
  }

But, while we are on the topic of style, it's

  if (something)

and

  if (!something)

Never

  if (something== true)

or

  if (something== false)
James Curran
What makes that style unspeakable?
Ken Ray
James Curran
People doing if(something == true) drives me up the wall, it's totally redundant.
jonnii
As far as braces go, I'm with Ken - I find the unspeakable to be just fine. I'm with jonni on comparing things to `true`, but more than just redundant - it's a bug waiting to happen.
Michael Burr
I vary depending on the on whether the boolean variable name sounds like it would make sense in an English sentence, but "something" sounds more like a variable than a condition. (I don't consciously think about it, that's just what my brain does with it)
Davy8
You have to name your booleans something intuitive like "isSomething" so when you "read" it in your mind it makes sense.
Chet
In a language that doesn't have a native boolean type, such as C, then if(something) is ambiguous. This is why it is not recommended.
staticsan
A: 

I would recommend that you do like Stephen and Nicholas Mancuso told.

Use:

if(something) {   DoSomething(); }

With or without the bracket. As soon as you start using weird version of "if" statement, you will starting seeing your collegues looking at your code in a weird way.

I normally use one liner for validation.

Example:

if( param1 == null ) throw new ArgumentNullException("param1");
Maxim
A: 

As long as you're consistent it shouldn't be an issue. At a previous company that was the typical scenario, but at my current company they prefer to have the braces on seperate lines.

Whytespot
+1  A: 

I just ran into this problem yesterday while working on code written by somebody else. The original code was:

if (something == true) 
    DoSomething();

and I wanted a debug print before calling DoSomething(). What I'd do instinctively

if (something == true) 
    print("debug message");
    DoSomething();

But that would make the if apply only to the debug message, while DoSomething() would be called unconditionally. That's why I'd rather have curly braces, so that the instinctive edit ends up as:

if (something == true) {
    print("debug message");
    DoSomething();
}
Nathan Fellman
So add the braces when you add the debug print. No reason to have them around if it isn't necessary.
Hawker
Good point, except that when I add a debug print I don't want to worry about braces too. I want to make as few changes as possible and still get the debug info without breaking the program.
Nathan Fellman
+1  A: 

The whitespace in form of new lines, indentation, spacing, alignment and so on is an important aspect of typography and is widely used to improve readability of the text in articles, books and web sites. Not sure why it won't apply the same to the readability of code.

Having said that, there's nothing wrong with you and your team using your style. As long as all of you agree on it.

Franci Penov
+10  A: 

I tend to put opening braces on their own line like this:

if (condition)
{
   statement;
   statement;
}

So seeing something like:

if (condition)
   statement;
   statement;

stands out as wrong right away. If I only have one statement I just leave it as

if (condition)
   statement;

and put the braces in later if I have an extra statement to add. I don't really see any room for confusion.

Putting the statement on the same line as the condition is a bad habit to get into, since when you're debugging, most debuggers count the whole thing as one line. (I realize that in C# this is not the case).

Eclipse
+6  A: 

Many people have suggested putting both on a single line. This may increase readability but at the cost of decreased debug-ability in my opinion. I've stepped through a lot of code written this way and it is more difficult to debug because of it.

Some debuggers and IDEs might be able to step over both parts of a single line if-statement and clearly show whether the condition evaluated true or not but many other debuggers may treat it as a single line making it difficult to determine whether the body of the if-statement was called.

For example the VS2008 debugger for C++ code will step over this as a single line making it difficult to determine whether Foo() was called.

if (a==b) { Foo(); }
Brian Ensink
+4  A: 

Personally, I like all my blocks to have the same pattern. I always use braces for ifs and they always start a new line. I like the idiom for automatically define public properties of putting the { get; set; } on the same line. I just feel that having all blocks start with a brace on its own line improves readability. As others have pointed out, it also makes it clearer in the debugger if you are stepping over lines.

If you disagree, then that's ok, too, but as others have said be consistent. To that end, you might want to share the "code formatting" settings between you and your co-workers so that the automatic formatting makes everything consistent for everyone.

I would do:

if (something)
{
   DoSomething();
}

and

public string MyProperty { get; set; }
tvanfosson
A: 

As long as you do it consistently, and make sure everyone working on your code knows how you do it, it doesn't matter what you do.

Just do what you find most comfortable, then make sure everyone knows to always do it that way.

Eli
A: 

Actually, I much prefer

if (something == true)
if (something == false)

over

if (something)
if (!something)

For me, the exclamation point is difficult to see at a glance, so is easy to miss. Note, however, that when I code in Python, I almost always prefer:

if something:
if not something:

Unless I want to distinguish None from, e.g., empty list.

Cybis
I've worked in companies that would near-sack anyone who wrote code with if (something == true) in it. The reason is that the bit value of true can vary from language to language and machine to machine. If you then pass that true out to another app in binary form, your expression style can fail.
David Arno
Interesting... that's something I hadn't considered before but you're right. However, how often do you export raw binary data like that? It seems like something that one would rarely do (unless perhaps your coding in C). Isn't using a serialization library typically a better choice?
Cybis
20 years ago, it wasn't that uncommon to interconnect say C and Fortran via such a binary method. These days though you are right, serialization is the norm, and such considerations ought not to be of concern. Your approach probably does improve readability, but keep in mind the (slight) downside.
David Arno
something == true is fundamentally flawed. You're basically saying, I want to explicitly ask whether something is true. But then, you're implicitly asking whether (something == true) is true. So you might as well say (something == true) == true, and why stop there?
Matthew Flaschen
Matthew, it isn't "fundamentally flawed". If I was programming in C, would you say "if (x != 0)" is fundamentally flawed because "if (x)" is equivalent? Besides, I was arguing readability, not semantics.
Cybis
A: 

I do occasionally like to do the

if(obj != null) obj.method();

But it's a guilty pleasure... I don't think it makes the code any more readable, so then why not follow a pattern everyone else is using. The one exception that I think is quite important is when you are showing how it's part of a bigger pattern, and helping the user see the pattern more easily:

public executeMethodOn(String cmd) {
  CommandObject co;

  if("CmdObject1".equals(cmd)) co=new CmdObject1();
  if("CmdObject2".equals(cmd)) co=new CmdObjec21();

  co.executeMethod();
}

It makes the pattern much more obvious and helps people trying to insert new functionality see where it needs to go.

That said, if you ever have a pattern like that, you are probably doing it wrong. I had to do this on a system that didn't have reflection, but I tried REALLY HARD to work around it, and if I'd had reflection, it would have been way awesomer.

Bill K
A: 

I usually do this with one-line ifs:

if($something) {
    do_something();
}

The one exception (I do Perl, not sure if this style is allowed in C#) is for loop controls, where I use the inverted one line style:

THING:
for my $thing (1 .. 10) {
    next THING if $thing % 3 == 0;
}

With good syntax coloring it's possible to make those lines stand out sharply.

Berserk
A: 

You're all WRONG WRONG WRONG!!!!! ;-)

Fortunately VS reformats all of your craziness into my personally approved format just by replacing the last curly brace in each file.

Whitespace is a style thing. Since we have different reading styles and learning styles, it really doesn't matter how you do it anymore. The tools will let us switch back and forth. The only downside to this that I have ever noticed is the toll it takes on tracking changes in source control. When I reformat a K&R style file into a more sane format (my opinion) and check that change back into source control, it shows almost every line as having changed. That's a pain. But many diff utilities can ignore whitespace changes (although most only on a single line, not spanning lines). That is an issue. But not a show stopper.

dviljoen
A: 

C gives a lot of freedom in formatting issues, so having them in the same line or in different lines is irrelevant for compiling purposes.

As a result, that "would it be bad" only refers to coding conventions. It depends on the context. If you work on a team, it is a good idea to ask everyone else what they think and come up with a standard formatting style. If not, it's really up to you.

So, if you think that it's better, go ahead. If not, then don't. It's really that simple(unless you use an IDE which imposes some styling conventions)

luiscubal
A: 

I prefer the horridly unspeakable syntax:

if (true == something) {
    doSomething1();
}

Yes, that is the way K&R did it...and yes, I go back that far...and yes, that is good enough reason for me. This means that I get common syntax even when I do something like this:

if (-1 == doSomething()) {
   doSomethingElse();
}

I put the curly braces in regardless of the number of the lines of code they contain. I have been bitten by the "need to add a line of test code" and missed the lack of curly braces one too many times.

I always compare with the literal on the left. This avoids the "testing an assignment" bug (e.g. if (something = true) {...}).

There is no more danger to the portability of "(something == true)" than there is with the overloaded set of values that mean "true" and "false" in a boolean comparison - but it is a different kind of danger. Are you writing in a language that considers "empty" (and/or zero and/or NULL and/or whitespace) to be "true" or "false"? I prefer a convention that is safe for every case...because I am lazy.

semiuseless
This is usually what you will see in Perl programs.
Brad Gilbert