CODEWITHSUNDEEP
c#language-agnosticcoding-style
David Ly
David Ly

Reputation: 31596

Would it be bad form to put braces on the same line as the statement for single line "if" statements?

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.

Upvotes: 8

Views: 3589

Answers (21)

Stan Graves
Stan Graves

Reputation: 6955

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.

Upvotes: 0

Nathan Fellman
Nathan Fellman

Reputation: 127498

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();
}

Upvotes: 2

luiscubal
luiscubal

Reputation: 25121

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)

Upvotes: 0

dviljoen
dviljoen

Reputation: 1632

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.

Upvotes: 0

Berserk
Berserk

Reputation: 433

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.

Upvotes: 0

Bill K
Bill K

Reputation: 62789

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.

Upvotes: 0

Cybis
Cybis

Reputation: 9863

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.

Upvotes: 0

Eli
Eli

Reputation: 99428

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.

Upvotes: 0

florin
florin

Reputation: 14326

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.

Upvotes: 3

tvanfosson
tvanfosson

Reputation: 532515

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; }

Upvotes: 6

Brian Ensink
Brian Ensink

Reputation: 11218

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(); }

Upvotes: 6

Eclipse
Eclipse

Reputation: 45493

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).

Upvotes: 9

Franci Penov
Franci Penov

Reputation: 76001

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.

Upvotes: 1

Whytespot
Whytespot

Reputation: 951

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.

Upvotes: 0

Maxime Rouiller
Maxime Rouiller

Reputation: 13699

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");

Upvotes: 0

James Curran
James Curran

Reputation: 103525

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)

Upvotes: 3

jonnii
jonnii

Reputation: 28312

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.

Upvotes: 6

DOK
DOK

Reputation: 32841

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();

Upvotes: 0

Timothy Khouri
Timothy Khouri

Reputation: 31845

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.

Upvotes: 0

Stephen Walcher
Stephen Walcher

Reputation: 2575

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.

Upvotes: 20

Nicholas Mancuso
Nicholas Mancuso

Reputation: 11887

Instead of:

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

Do this:

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

Upvotes: 10

Related Questions