73
if   i>0 : return sqrt(i)  
elif i==0: return 0  
else     : return 1j * sqrt(-i)

VS

if i>0:  
   return sqrt(i)  
elif i==0:  
   return 0  
else:  
   return 1j * sqrt(-i)  

Given the above examples, I don't understand why I virtually never see the first style in code bases. To me, you turn the code into a tabular format that shows clearly what you want. First column can virtually be ignored. Second column identifies the condition and the third column gives you the output you want. It seems, at least to me, straight-forward and easy to read. Yet I always see this simple kind of case/switch situation come out in the extended, tab indented format. Why is that? Do people find the second format more readable?

The only case where this could be problematic is if the code changes and gets longer. In that case, I think it's perfectly reasonable to refactor the code into the long, indented format. Does everyone do it the second way simply because it's the way it always was done? Being a devil's advocate, I guess another reason might be because people find two different formats depending upon complexity of the if/else statements to be confusing? Any insight would be appreciated.

  • 91
    Because people find the 2nd option more readable? – GrandmasterB Jul 26 '16 at 19:36
  • 65
    The use case of mutually-exclusive branches that all return a value of the same type doesn't come up often in imperative languages compared to branches that may not return values, might span multiple lines and probably have side effects. If you looked at functional programming languages you'd see code that resembles your first example much more often. – Doval Jul 26 '16 at 19:50
  • 47
    @horta "The only case where this could be problematic is fi the code changes and gets longer." -- You should NEVER assume that a piece of code won't be changed. Code refactoring takes up a large majority of a software's lifecycle. – Charles Addis Jul 26 '16 at 20:14
  • 7
    @horta: Nothing to do with me. It's the code. In the context of the codebase I'm reading, I want to see if statements (and other language constructs) consistently formatted, without any edge cases. It's not that I learned to read code in a particular way, I can read either just fine, but it's more readable if everything is the same. Again, not the same to me, the same to the rest of the code. – GManNickG Jul 26 '16 at 23:31
  • 44
    Also, most debuggers are line based. Can't put a breakpoint on a statement that's inside an if if it's on the same line. – isanae Jul 27 '16 at 1:01

10 Answers 10

93

One reason may be that you're not using languages where it's popular.

A few counter-examples:

Haskell with guards and with patterns:

sign x |  x >  0        =   1
       |  x == 0        =   0
       |  x <  0        =  -1

take  0     _           =  []
take  _     []          =  []
take  n     (x:xs)      =  x : take (n-1) xs

Erlang with patterns:

insert(X,Set) ->
    case lists:member(X,Set) of
        true  -> Set;
        false -> [X|Set]
    end.

Emacs lisp:

(pcase (get-return-code x)
  (`success       (message "Done!"))
  (`would-block   (message "Sorry, can't do it now"))
  (`read-only     (message "The shmliblick is read-only"))
  (`access-denied (message "You do not have the needed rights"))
  (code           (message "Unknown return code %S" code)))

Generally I see the table format is pretty popular with functional languages (and in general expression based ones), while breaking the lines is most popular in others (mostly statement based).

  • 10
    I upvoted this and generally agree, but feel obligated to point out that 1. All of these are trivial returns 2. Haskellers are fond of comically short identifiers in addition to the terseness of the language itself and 3. Functional languages tend to be expression based, and even imperative languages have different standards for expressions than statements. If the OP rewrote the original example as function applications he might get different advice... – Jared Smith Jul 27 '16 at 18:03
  • 3
    @JaredSmith Thanks for the expression/statement based split - I think it may be even more fitting than functional/imperative. Than again ruby is almost expression based and doesn't use that convention often. (exceptions to everything) Regarding points 1 and 2, I find 50%+ of real Haskell code to be "trivial returns" that are just parts of something bigger - it's just how that code is written - not only in examples here. For example close to half of the functions here are one just one/two liners. (some lines use table layout) – viraptor Jul 27 '16 at 22:40
  • Yes. I'm not a Haskeller but do some ocaml and find that pattern matching tends to be a lot more concise than logically equivalent switches, and polymorphic functions cover a lot of that ground anyways. I imagine Haskell's type classes would expand that coverage even further. – Jared Smith Jul 27 '16 at 23:34
  • I think it's the pattern case syntax that promotes that. Because it's more concise and usually closer to a short switch-case, it's easier to express as one-liners. I frequently do this with short switch-case statements as well for similar reasons. Literal if-else statements are still usually spread across multiple lines when not effectively a simple ternary, though. – Isiah Meadows Jul 30 '16 at 0:28
  • @viraptor Technically the other 50%- of haskell code is "non-trivial returns" because all haskell functions are functionally pure and can't have side effects. Even functions that read from and print to the command line are just lengthy return statements. – Pharap Jul 30 '16 at 18:02
134

It's more readable. A few reasons why:

  • Nearly every language uses this syntax (not all, most - your example appears to be Python, though)
  • isanae pointed out in a comment that most debuggers are line based (not statement based)
  • It starts looking even more ugly if you have to inline semicolons or braces
  • It reads top to bottom more smoothly
  • It looks horribly unreadable if you have anything other than trivial return statements
    • Any indenting meaningful syntax is lost when you skim code as the conditional code is no longer visually separated (from Dan Neely)
    • This will be particularly bad if you continue to patch/add items into the 1-line if statements
  • It's only readable if all your if checks are about the same length
  • It means you cannot format complicated if statements into multiline statements, they will have to be oneliners
  • I am far more likely to notice bugs/logicflow when reading vertically line by line, not trying to parse multiple lines together
  • Our brains read narrower, taller text MUCH faster than long, horizontal text

The minute you try to do any of this you will end up rewriting it into multiline statements. Which means you just wasted time!

Also people inevitably add something like:

if i>0:  
   print('foobar')
   return sqrt(i)  
elif i==0:  
   return 0  
else:  
   return 1j * sqrt(-i)  

It doesn't take very often doing this before you decide this format is a lot better than your alternative. Ah, but you could inline it all in one line! enderland dies on the inside.

Or this:

if   i>0 : return sqrt(i)  
elif i==0 and bar==0: return 0  
else     : return 1j * sqrt(-i)

Which is really, really annoying. No one likes formatting things like this.

And last, you will start the holy war of "how many spaces for tabs" problem. What renders perfectly on your screen as a table format may not render on mine depending on settings.

Readability should not depend on IDE settings anyways.

  • 14
    @horta because you will have to convert it if you do format it your first way, initially? I'm all about minimizing work for future enderland. Making pretty whitespace tables and counting spaces and tabs to update the visual formatting when I might add some logic to an if check isn't fun at all (ignoring the readability implications). – enderland Jul 26 '16 at 20:33
  • 14
    @horta He's not necessarily saying he wouldn't fix it, he's saying he'd have to fix it, and that's a lot of time spent on tedious formatting, rather than on programming. – Servy Jul 26 '16 at 20:39
  • 11
    @horta: IMHO your way is often less readable, and is definitely a lot more annoying to format. Also, it can only be used when the "ifs" are small one liners, which is a seldom case. Lastly, it's somehow not nice, IMHO, to carry two kind of "if" formattings around because of this. – dagnelies Jul 26 '16 at 20:54
  • 9
    @horta you seem to be blessed with working on systems where requirements, systems, APIs, and user needs never change. You lucky soul. – enderland Jul 26 '16 at 22:02
  • 11
    I'd add: any small change in a single condition may require reformatting the others to match the : position -> doing a diff in a CVS it suddenly becomes more difficult to understand what is actually changin. This is also true for condition vs body. Having them on separate lines means that if you change only one of them the diffs clearly show that only the condition changed, not the body. – Bakuriu Jul 26 '16 at 22:20
55

I am a firm believer in 'code is read many times, written few -- so readability is very important.'

A key thing that helps me when I read other people's code is that is follows the 'normal' patterns that my eyes are trained to recognize. I can read the indented form most easily because I've seen it so many times that it registers almost automatically (with little cognitive effort on my part). It's not because it's 'prettier' -- it's because it follows the conventions that I'm used to. Convention beats 'better' ...

  • 3
    Relevant: thecodelesscode.com/case/94 – Kevin Jul 27 '16 at 0:57
  • 11
    This explains why people are conservative. It does not explain why people chose to write their code a certain way to begin with. – Jørgen Fogh Jul 27 '16 at 8:31
  • 8
    The question was 'why do I see this often', not where did this style come from. Both questions are interesting; I tried to answer the one I thought was being asked. – Art Swri Jul 27 '16 at 15:01
16

Along with the other drawbacks already mentioned, tabular layout increases the odds of version control merge conflicts requiring manual intervention.

When a block of tabularly alined code needs to be realigned, the version control system will treat each of those lines as having been modified:

diff --git a/foo.rb b/foo.rb
index 40f7833..694d8fe 100644
--- a/foo.rb
+++ b/foo.rb
@@ -1,8 +1,8 @@
 class Foo

   def initialize(options)
-    @cached_metadata = options[:metadata]
-    @logger          = options[:logger]
+    @metadata = options[:metadata]
+    @logger   = options[:logger]
   end

 end

Now suppose that in the mean time, in another branch, a programmer has added a new line to the block of aligned code:

diff --git a/foo.rb b/foo.rb
index 40f7833..86648cb 100644
--- a/foo.rb
+++ b/foo.rb
@@ -3,6 +3,7 @@ class Foo
   def initialize(options)
     @cached_metadata = options[:metadata]
     @logger          = options[:logger]
+    @kittens         = options[:kittens]
   end

 end

Merging that branch will fail:

wayne@mercury:/tmp/foo$ git merge add_kittens
Auto-merging foo.rb
CONFLICT (content): Merge conflict in foo.rb
Automatic merge failed; fix conflicts and then commit the result.

If the code being modified had not used tabular alignment, the merge would have succeeded automatically.

(This answer was "plagiarized" from my own article discouraging tabular alignment in code).

  • 1
    Interesting, but isn't this a failing of merge tools? Specifically git in this case? This is a datapoint towards convention being the easy way to go. To me, it's something that could be improved upon (from the tool side). – horta Jul 28 '16 at 22:19
  • 7
    @horta For the merge tool to modify white space in a way that doesn't almost always break the code, it will have to understand in what ways it can modify white space without changing the meaning of the code. It will also have to understand the particular tabular alignment being used. That will be not only language dependent (Python!), but probably require the tool to understand the code to some degree. On the other hand, line based merging can be done without AI, and quite often does not even break the code. – Wayne Conrad Jul 28 '16 at 22:23
  • Gotcha. So as mentioned elsewhere in the comments, until we have IDE's or programming input formats that incorporate tables directly into the format, tooling problems will always be there just making life difficult for those of us who prefer tables. – horta Jul 28 '16 at 22:32
  • 1
    @horta Correct. Most of my objections to tabular alignment in code could go away with sufficiently advanced tools. – Wayne Conrad Jul 28 '16 at 22:36
8

Tabular formats can be very nice if things always fit within the allotted width. If something exceeds the allotted width, however, then it often becomes necessary to either have part of the table that isn't lined up with the rest of it, or else adjust the layout of everything else in the table to fit match the long item.

If source files were edited using programs that were designed to work with tabular-format data and could handle overly long items by using a smaller font size, splitting them into two lines within the same cell, etc. then it might make sense to use tabular formats more often, but most compilers want source files that are free of the kinds of markup that such editors would need to store in order to maintain formatting. Using lines with variable amounts of indent but no other layout isn't as nice as tabular formatting in the best case, but it doesn't cause nearly as many problems in the worst case.

  • True. I have noticed the text editor I use (vim) has horrible support for tabular formatting or even wide text. I haven't seen other text-editors do much better. – horta Jul 27 '16 at 12:15
6

There is the 'switch' statement that provides this kind of thing for special cases, but I guess that's not what you're asking about.

I have seen if statements in table format, but there has to be a large number of conditions to make it worthwhile. 3 if statements are best shown in the traditional format, but if you had 20, then its much easier to display them in a big block that's formatted to make it clearer.

And there's the point: clarity. If it makes it easier to see (and your first example is not easy to see where the : delimiter is) then format it to suit the situation. Otherwise, stick with what people expect, as that's always easier to recognise.

  • 1
    OP appears to be using Python, so no switch. – Jared Smith Jul 27 '16 at 18:08
  • 2
    "3 if statements are best shown in the traditional format, but if you had 20" ... then you have bigger issues to be thinking about! : ) – Grimm The Opiner Jul 28 '16 at 12:12
  • @GrimmTheOpiner If you're writing a language parser or AST stringifier, that's a very possible thing to deal with. For example, I once contributed to a JavaScript parser where I split up a function with 15-20 cases, one for each expression type. I segmented most cases off to their own functions (for a notable perf boost), but the long switch was a necessity. – Isiah Meadows Jul 30 '16 at 0:39
  • @JaredSmith: Apparently switch is evil, but instantiating a dictionary then performing a lookup on it in order to do trivial branching isn't evil... – Mark K Cowan Jul 30 '16 at 18:47
  • 1
    @MarkKCowan oh I caught the sarcasm but thought you were using it to mock me. lack of context on the internet and whatnot. – Jared Smith Jul 31 '16 at 14:46
1

If your expression really is that easy most programming languages offer the ?: branching operator:

return  ( i > 0  ) ? sqrt( i)
      : ( i == 0 ) ? 0
        /* else */ : 1j * sqrt( -i )

This is a short readable tabular format. But the important part is: I see at one glance what the "major" action is. This is a return statement! And the value is decided by certain conditions.

If on the other hand you have branches which execute different code, I find it a lot more readable to indent these blocks. Because now there are different "major" actions depending on the if-statement. In one case we throw, in one case we log and return or just return. There is a different program flow depending on the logic, so code-blocks encapsulate the different branches and make them more prominent for the developer (e.g. speed-reading a function to grasp the program flow)

if ( i > 0 )
{
    throw new InvalidInputException(...);
}
else if ( i == 0 )
{
    return 0;
}
else
{
    log( "Calculating sqrt" );
    return sqrt( -i );
}
  • 7
    Actually, I find your "short readable tabular format" quite a nightmare to read, while the format proposed by OP is perfectly fine. – Matteo Italia Jul 27 '16 at 11:22
  • @MatteoItalia how about this edited version ? – Falco Jul 27 '16 at 11:27
  • 5
    Sorry, still worse; I think that it comes from the fact that the ? and : are harder to spot than the if/else keywords and/or due to the added "noise" of the symbols. – Matteo Italia Jul 27 '16 at 12:33
  • @MatteoItalia: I've had cases with over hundred different values. The table value makes it possible to check for errors. With multi-line, it's impossible. – gnasher729 Jul 27 '16 at 14:47
  • 1
    @gnasher729 - For 100 different "values" I have generally found it to be much better to declare a data structure of each "item" and list it all out as a table in the initialization for an array of these data structures. (Of course language limitations may apply here). If any item requires a "computational" aspect the item structure can contain a pointer or reference to a function to perform the needed action. For may applications this can greatly simplify code and make maintenance much easier. – Michael Karas Jul 28 '16 at 11:40
1

As enderland has already said, you're assuming you only ever have one "return" as the action, and that you can tag that "return" onto the end of the condition. I'd like to give some extra detail for why this isn't going to be successful.

I don't know what your preferred languages are, but I've been coding in C for a long time. There are a number of coding standards around which aim to avoid some standard coding errors by disallowing code constructions which are prone to errors, either in initial coding or during later maintenance. I'm most familiar with MISRA-C, but there are others, and generally they all have similar rules because they're addressing the same problems in the same language.

One popular error which coding standards often address is this little gotcha:-

if (x == 10)
    do_something();
    do_something_else();

This does not do what you think it does. As far as C is concerned, if x is 10 then you call do_something(), but then do_something_else() gets called regardless of the value of x. Only the action immediately following the "if" statement is conditional. This may be what the coder intended, in which case there's a potential trap for maintainers; or it may not be what the coder intended, in which case there's a bug. It's a popular interview question.

The solution in coding standards is to mandate braces around all conditional actions, even if they're single-line. We now get

if (x == 10)
{
    do_something();
    do_something_else();
}

or

if (x == 10)
{
    do_something();
}
do_something_else();

and now it works correctly and is clear for maintainers.

You will notice that this is entirely incompatible with your table-style format.

Some other languages (e.g. Python) looked at this problem and decided that since coders were using whitespace to make layout clear, it would be a good idea to use whitespace instead of braces. So in Python,

if x == 10:
    do_something()
    do_something_else()

makes the calls to both do_something() and do_something_else() conditional on x == 10, whereas

if x == 10:
    do_something()
do_something_else()

means that only do_something() is conditional on x, and do_something_else() is always called.

It's a valid concept, and you'll find a few languages use it. (I first saw it in Occam2, way back when.) Again though, you can easily see that your table-style format is incompatible with the language.

  • 1
    I think you missed the point. The issue you speak of is a weird C specific non-standard nightmare that causes the issue you speak of. If coding in C, I would never recommend using the alternate simple if method with the tabular format I suggested. Instead, since you're using C, you would use braces all on the single line. The braces would actually make the table format even more clear because they're acting as delimiters. – horta Jul 27 '16 at 12:23
  • Also, the return statements in this case is just an example. In general, that may be code smell in itself. I'm just referring to the format of simple statements, not necessarily with return statements at all. – horta Jul 27 '16 at 12:31
  • 2
    My point was that this makes a table format even more clunky. Incidentally, it's not C-specific - it's shared by all languages derived from C, so C++, C#, Java and JavaScript all allow the same gotcha. – Graham Jul 27 '16 at 15:25
  • 1
    I don't mind that it's return statements - I get that your intention is to show simple statements. But it becomes more cumbersome. And of course as soon as any statement becomes non-simple, then you need to change the formatting because a table format is impossible to maintain. Unless you're doing code obfuscation, long lines of code are a smell in their own right. (Original limit was 80 chars, these days it's more typically around 130 chars, but the general principle still holds that you shouldn't have to scroll to see the end of the line.) – Graham Jul 27 '16 at 15:28
1

Tabular layout can be useful in few limited cases, but there are few times it is useful with if.

In simple cases ?: may be a better choice. In medium cases a switch is often a better fit (if your language has one). In complicated cases you might find that a call tables are a better fit.

There have been many times when refactoring code that I have rearranged it to be tabular to make it's patern obvious. It is seldom the case that I leave it that way in that in most cases there is a better way to solve the problem once you understand it. Occasionally a coding practice or layout standard prohibits it, in which case a comment is useful.

There was some questions about ?:. Yes it is the ternary operator (or as I like to think of it the value if). on first blush this example is a little complicated for ?: (and overusing ?: does not help readability but hurts it), but with some thought The example can be rearranged as below, But I think in this case a switch is the most readable solution.

if i==0: return 0
return i>0?sqrt(i):(1j*sqrt(-i))
  • 1
    You may have to make it clear what "?:" is for the uninitiated (for instance, with an example, possibly related to the question). – Peter Mortensen Jul 27 '16 at 9:05
  • I'm assuming that's the ternary operator. I feel like that's shunned for the good reason that the ternary operator tends to rearrange the standard if, do stuff, else, do other stuff format that people see day in and day out and therefore can read easily. – horta Jul 27 '16 at 12:27
  • @PeterMortensen: If the uninitiated don't know what this means, they should stay away from the code until they asked the obvious question and learned. – gnasher729 Jul 27 '16 at 14:49
  • @horta Ternary is if ? do stuff : do other stuff. Same order as an if/else. – Navin Jul 27 '16 at 15:28
  • 1
    @Navin Ah, maybe it's just a failure of the language I use most (python). stackoverflow.com/questions/394809/… – horta Jul 27 '16 at 15:31
-3

I don't see anything wrong with the table format. Personal preference, but I would use a ternary like this:

return i>0  ? sqrt(i)       :
       i==0 ? 0             :
              1j * sqrt(-i)

No need to repeat return every time :)

  • 1
    As mentioned in a few comments, the return statements isn't ideal or the point of the post, simply a chunk of code I found online and formatted a couple ways. – horta Jul 27 '16 at 15:35
  • Python ternaries are do_something() if condition() else do_something_else(), not condition() ? do_something() : do_something_else(). – Isiah Meadows Jul 30 '16 at 0:42
  • @IsiahMeadows OP never mentioned Python. – Navin Jul 30 '16 at 0:43

protected by gnat Jul 27 '16 at 19:34

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.