2

When reviewing code, I prefer to read if statements that explicitly demonstrate that a fork in code execution may occur:

if ( isFoo(baz) ) {
    runBar();
}

When scanning hundreds of lines of the nonindented version, it is not obvious that a conditional may or may not occur:

boom += 1;
baz = getBaz(boom);
if (isFoo(baz)) runBar();
boing = boom + baz;

So when is it considered all right to single-line if statements? I'm willing to put up with it if the conditional is on a die(), return, or exception, because in those cases at least the current method ends.

if (isFoo(baz)) die("No more foo!");

Are there any other "good" uses for single-line if statements? How strictly should the practice be avoided? Note that I am explicitly asking about the lack of an indented line to indicate that a condition may or may not occur, I am not asking about the use of braces. Although I would appreciate all opinions in the comments, answers should address objective reasoning, such as maintainability or extensibility of code.

This is not a dupe of Single statement if block - braces or no because this question does not ask about braces. This is not a dupe of Single Line Statements & Good Practices because that question does not address the crux of this question: the ability to determine that some lines of code may or may not be run, thus leading to divergent code paths.

dotancohen
  • 1,041
  • see Single Line Statements & Good Practices. Your gain is saving one line once when writing code, your pain is a broken "flow" every time after that, when you or someone else reads it – gnat Jul 01 '15 at 13:49
  • 3
    I see your point. However, the answers there might answer your question; this is something primarily opinion-based. – mike Jul 01 '15 at 13:54
  • 2
    Definitely subjective. I use them to check for nulls, and very simple/clear statements, just because I feel like conserving space. And who doesnt like elegant one liners:
    if(obj == null) break;
    – CodyF Jul 01 '15 at 14:41
  • 2
    "When to use single-line if statements?" Never. (please!) – utnapistim Jul 01 '15 at 14:44
  • 3
    @CodyF, And who doesnt like elegant one liners: if(obj == null) break; I don't; every time I find one in the current code base, it's a reading interruption (as in "WTF?! oh! same line!"). I have checked out more than a few files, just to fix the damn same-line ifs and commit again. – utnapistim Jul 01 '15 at 14:47
  • It comes down to style. I've never seen the consensus towards this formalised into some sort of standard. My 'style' is to always use braces, if only for clarity. The use of which, I've found, has been a result of people trying to be too concise. No need. – JᴀʏMᴇᴇ Jul 01 '15 at 15:37
  • 3
    I'm not convinced either duplicate link is relevant when you read past their titles. – gbjbaanb Jul 01 '15 at 15:53
  • 2
    @gbjbaanb: You are right. In fact, I've already asked on meta to reopen this. Please mention your observation on that thread, thank you! – dotancohen Jul 01 '15 at 15:56
  • 2
    In the case o C, I prefer ()?():() instead of single line if's, because this construct is expected to use just one line. I got confused with my own code after a while when I used single line if's. – Mandrill Jul 02 '15 at 16:10
  • High-level issues like single-responsibility principle and brevity of functions are more important than this style issue. Think about it this way: if your routine is simple and has a clear purpose, does it really matter whether you write if (foo) bar() or if (foo) { <newline> bar(); <newline> }. Your routines are probably just too complicated or unclear in responsibility. And instead of identifying high-level issues, you basically will tend to automatically look for the little things like formatting of if statements and assume that that's why the code is bad. It's not. – Brandin Jul 02 '15 at 21:38
  • Why the sudden rash of downvotes? If the question can be improved, then let me know how! Thanks. – dotancohen Jul 04 '15 at 15:53
  • given that you asked about it at meta, increase in voting is likely due to "meta efect" – gnat Jul 05 '15 at 12:10
  • 1
    I am astonished that there is not one single mention of the word "breakpoint" on this page. If you are debugging and only want to stop when the action is taken, how will you do that if it is all on a single line? And if your stop on the single line and one-step, how do you know if any action was taken or not? – Mawg says reinstate Monica Jul 27 '15 at 13:27
  • Many modern debuggers allow mid-line breakpoints and will highlight the portion of the line being executed as you step through. – snarf Sep 06 '20 at 14:52

3 Answers3

15

The only time to use a single-line if statement is when you have a lot of them and you can format your code to make it very clear what is happening. Anything else is not clear and will lead to errors.

eg.

if (x) do_stuff();

is terrible. When mixed in with other code, it does not clearly show the conditional statement, and some people will confuse the following line as a badly-indented conditional. This applies to all statements, especially if they are die(), return or throw. It has nothing to do with the running of the code, its all about the readability.

if (a) do_a();
if (b) do_b();
if (c) do_c();

this is much better than putting the conditionals on the next line as its clearer what is intended, there is no scope for confusion.

gbjbaanb
  • 48,585
  • 6
  • 103
  • 173
  • 13
    I would contend that even for single line if, to avoid future errors one should consider if (a) { do_a(); } *with* the braces. Yes, they're unnecessary but it will avoid static analysis warnings and make it harder to make a mistake in the future. –  Jul 01 '15 at 13:31
  • 1
    that would usually be the argument to use a statemachine switch – Peter Jul 11 '23 at 06:20
11

I always prefer the version with braces.

  1. If I don't always use the braces, then I may forget to put them when there is more than one statement in the block.
  2. IDEs used by other team members can reformat the code automatically to move the statement to the new line, thus making it much more error prone in the future.
  3. Merging the code across the branches is more error prone if the if statement is a conflicting spot. The probability for error is even higher if the person doing the merge is not completely familiar with the merged changes.
  4. A statement not-belonging to the block may be included in it later by mistake (I add new statements to the block, so I need to add braces, but because of indentation or similar visual effects I also include a statement that should be left out of the block).
6

I would propose an alternative answer. I prefer single liners when the condition inside is really a single line and is relatively isolated from the rest of the conditions.

One great example is:

public void DoSomething(int something) {
     // Notice how easily we can state in one line that we should exit the method if our int is 0.
     if(something == 0) return;

     CallAnotherMethodHere(something);
}

A bad example(in my opinion) would be what @gbjbaanb mentioned:

if (a) do_a();
if (b) do_b();
if (c) do_c();

To me, this is a switch statement and should be written as such:

switch(something) {
    case "a":
        do_something_to_a();
        break;
    case "b":
        do_something_to_b();
        break;
    case "c":
        do_something_to_c();
        break;
    default:
        break;
}

Much more readable IMHO and more efficient as variable needs to be evaluated only once VS running through all the if statements although gains are negligible.

Alexus
  • 2,388
  • 1
    Thank you. The OP also mentions that return statements are an exception that should be allowed, as the method ends there. – dotancohen Jul 02 '15 at 16:41
  • agreed... basically single line if are a bug trap with marginal benefits. Personally I do them for argument validation, everything else, if it must be single-line I prefer the trinary operator or go all the way with a proper if with braces and all – Newtopian Jul 02 '15 at 16:41
  • 4
    Your example assumes that a,b, and c are mutually exclusive. – Brian Jul 02 '15 at 16:54
  • 2
    @Brian You are correct, because if they are not, I would start looking at the design because your method is responsible for a bunch of operations, and that's not a good sign. – Alexus Jul 02 '15 at 17:21
  • 2
    Ugh... you prefer 13 verbose lines to 3? (Which, by the way, are far more readable due to the obvious repetition structure. And there's no chance of misplaced breaks.) Efficiency is a non-issue since optimizers are usually smart enough for something this simple. – Mateen Ulhaq Nov 09 '18 at 21:30
  • @MateenUlhaq In case of a switch, program executes the statements until it hits the valid case. In case with ifs, the program will execute every single if. – Alexus Nov 15 '18 at 00:06
  • 1
    They're logically equivalent. This is simple enough that I'm sure an optimizing compiler would generate the same assembly for both. Maybe I'll test it out when I get home. – Mateen Ulhaq Nov 15 '18 at 02:38
  • @MateenUlhaq Most like once compiled they are indeed the same. But do test out. Preferably with a large amount of looped execution to see if there are any performance difference. – Alexus Nov 16 '18 at 23:57
  • 1
    This really depends on what you're testing - in @gbjbaanb's answer, the conditions could all be true, executing all three do_x functions, whereas the switch will only hit one. Also I'd format that simple switch so that each case is only on one line, and line up each statement within each case – Dutchmanjonny Jun 25 '20 at 08:44
  • nope....switch will replace if...else-if. single ifs can be satisfied by one or more conditions. – Abhiroj Panwar Aug 06 '20 at 15:48