26

I recently discovered that our company has a set of coding guidelines (hidden away in a document management system where no one can find it). It generally seems pretty sensible, and keeps away from the usual religious wars about where to put '{'s and whether to use hard tabs. However, it does suggest that "lines SHOULD NOT contain embedded multiple spaces". By which it means don't do this sort of thing:

foo    = 1;
foobar = 2;
bar    = 3;

Or this:

if      ( test_one    ) return 1;
else if ( longer_test ) return 2;
else if ( shorter     ) return 3;
else                    return 4;

Or this:

thing foo_table[] =
{
  { "aaaaa", 0 },
  { "aa",    1 },
  // ...
}

The justification for this is that changes to one line often require every line to be edited. That makes it more effort to change, and harder to understand diffs.

I'm torn. On the one hand, lining up like this can make repetitive code much easier to read. On the other hand, it does make diffs harder to read.

What's your view on this?

Linus Fernandes
  • 498
  • 5
  • 30
Ned
  • 2,142
  • 17
  • 24

14 Answers14

30

2008: Since I supervise daily merges of source code,... I can only recommend against it.

It is pretty, but if you do merges on a regular basis, the benefit of 'easier to read' is quickly far less than the effort involved in merging that code.

Since that format can not be automated in a easy way, the first developer who does not follow it will trigger non-trivial merges.

Do not forget that in source code merge, one can not ask the diff tool to ignore spaces :
Otherwise, "" and " " will look the same during the diff, meaning no merge necessary... the compiler (and the coder who added the space between the String double quotes) would not agree with that!

2020: as noted in the comments by Marco

most code mergers should be able to handle ignoring whitespace and aligning equals is now an auto format option in most IDE.

I still prefer languages which come with their own formatting options, like Go and its gofmt command.
Even Rust has its rustfmt now.

VonC
  • 1,262,500
  • 529
  • 4,410
  • 5,250
  • 3
    Isn't it even worse than that? Any edit which requires re-lining up (which in practice seems to be more often that you'd like) gives you a diff in which the whole block of code changes. That's going to mean a non-trivial merge right away. – Ned Sep 19 '08 at 14:08
  • 1
    Finally! I was beginning to think I was the only one thinking that ;) Even if @Agoston Horvath suggest to ignore spaces, not every tools used in versionning tool are able to do that, and it does, indeed, become "even worse than that" – VonC Sep 19 '08 at 14:54
  • 2
    Isn't this a failing in source control systems, then, if they cannot ignore spaces? – DisgruntledGoat Jul 06 '09 at 13:26
  • @DisgruntledGoat: I agree that this should be handled in the version system, but this has to mean the version system is aware of the meaning of the code. – xtofl Aug 28 '09 at 08:36
  • Note the above answer is still one of the first hits on Google. It was written in 2008, most code mergers should be able to handle ignoring whitespace and aligning equals is now an auto format option in most IDE. – Marco Jul 14 '20 at 07:39
  • @Marco I agree. I have included your comment in the answer for more visibility. – VonC Jul 14 '20 at 08:38
26

I'm torn. On the one hand, lining up like this can make repetitive code much easier to read. On the other hand, it does make diffs harder to read.

Well, since making code understandable is more important than making diffs understandable, you should not be torn.

IMHO lining up similar lines does greatly improve readability. Moreover, it allows easier cut-n-pasting with editors that permit vertical selection.

Eli Bendersky
  • 263,248
  • 89
  • 350
  • 412
  • A compromise would be to leave existing code alone but allow re-formatting for new code or code already "touched". There are other changes like reordering lists alphabetically or numerically that the arguement could apply to. – AlanKley Sep 19 '08 at 14:49
20

I never do this, and I always recommend against it. I don't care about diffs being harder to read. I do care that it takes time to do this in the first place, and it takes additional time whenever the lines have to be realigned. Editing code that has this format style is infuriating, because it often turns into a huge time sink, and I end up spending more time formatting than making real changes.

I also dispute the readability benefit. This formatting style creates columns in the file. However, we do not read in column style, top to bottom. We read left to right. The columns distract from the standard reading style, and pull the eyes downward. The columns also become extremely ugly if they aren't all perfectly aligned. This applies to extraneous whitespace, but also to multiple (possibly unrelated) column groups which have different spacing, but fall one after the other in the file.

By the way, I find it really bizarre that your coding standard doesn't specify tabbing or brace placement. Mixing different tabbing styles and brace placements will damage readability far more than using (or not using) column-style formatting.

Derek Park
  • 45,824
  • 15
  • 58
  • 76
  • I think it has a meta-rule about sticking to one style within a file/module/project. In any case, barely anyone even knows this document exists :) – Ned Sep 22 '08 at 08:09
  • 1
    I always find it odd when companies don't have global rules that they enforce. Allowing people to use whatever style they want means that moving code from one project to another becomes problematic. – Derek Park Sep 24 '08 at 00:07
  • If there's a really well-written set of functions in another project, but written in an entirely different style, it's problematic to drop them into my project. I either have to mix styles, or waste time reformatting the code. – Derek Park Sep 24 '08 at 00:08
  • How long does it take to invoke a pretty-printer? – jwg Aug 01 '13 at 11:23
16

I never do this. As you said, it sometimes requires modifying every line to adjust spacing. In some cases (like your conditionals above) it would be perfectly readable and much easier to maintain if you did away with the spacing and put the blocks on separate lines from the conditionals.

Also, if you have decent syntax highlighting in your editor, this kind of spacing shouldn't really be necessary.

Lucas Oman
  • 15,597
  • 2
  • 44
  • 45
12

There is some discussion of this in the ever-useful Code Complete by Steve McConnell. If you don't own a copy of this seminal book, do yourself a favor and buy one. Anyway, the discussion is on pages 426 and 427 in the first edition which is the edition I've got an hand.


Edit:

McConnell suggests aligning the equal signs in a group of assignment statements to indicate that they're related. He also cautions against aligning all equal signs in a group of assignments because it can visually imply relationship where there is none. For example, this would be appropriate:

Employee.Name  = "Andrew Nelson"
Employee.Bdate = "1/1/56"
Employee.Rank  = "Senator"
CurrentEmployeeRecord = 0

For CurrentEmployeeRecord From LBound(EmployeeArray) To UBound(EmployeeArray) 
. . .

While this would not

Employee.Name         = "Andrew Nelson"
Employee.Bdate        = "1/1/56"
Employee.Rank         = "Senator"
CurrentEmployeeRecord = 0

For CurrentEmployeeRecord From LBound(EmployeeArray) To UBound(EmployeeArray) 
. . .

I trust that the difference is apparent. There is also some discussion of aligning continuation lines.

Onorio Catenacci
  • 14,928
  • 14
  • 81
  • 132
  • 1
    Maybe you could provide a short quote or summarize the discussion to make the answer more helpful. – kaa Sep 30 '08 at 10:03
  • 8
    McConnell did a 180 on this recommendation in subsequent revisions of *Code Complete*. His advice in the second revision: *Do not align right sides of assignment statements*. He acknowledged a slight increase in readability and comprehensibility from such alignment, but that slight increase is not justified by the huge maintenance cost associated with this style. – David Hammen May 13 '14 at 16:01
  • @DavidHammen I'm glad you added that comment; I was totally unaware that he'd changed his suggestion on this point. – Onorio Catenacci Sep 16 '14 at 16:26
  • The relevant page in McConnell's book (second edition) is page 758. – agorf Dec 17 '17 at 11:23
8

Personally I prefer the greater code readability at the expense of slightly harder-to-read diffs. It seems to me that in the long run an improvement to code maintainability -- especially as developers come and go -- is worth the tradeoff.

Rylee Corradini
  • 733
  • 4
  • 13
3

With a good editor their point is just not true. :)

(See "visual block" mode for vim.)

P.S.: Ok, you still have to change every line but it's fast and simple.

Sarien
  • 6,647
  • 6
  • 35
  • 55
3

I try to follow two guidelines:

  1. Use tabs instead of spaces whenever possible to minimize the need to reformat.

  2. If you're concerned about the effect on revision control, make your functional changes first, check them in, then make only cosmetic changes.

Public flogging is permissible if bugs are introduced in the "cosmetic" change. :-)


2020-04-19 Update: My, how things change in a dozen years! If I were to answer this question today, it would probably be something like, "Ask your editor to format your code for you and/or tell your diff tool to ignore whitespace when you're making cosmetic changes.

Today, when I review code for readability and think the clarity would be improved by formatting it differently, I always end the suggestion with, "...unless the editor does it this way automatically. Don't fight your tools. They always win."

Adam Liss
  • 47,594
  • 12
  • 108
  • 150
  • 1
    I was going to recommend this. I usually do it the other way around. First cosmetic changes only. Gets me familiar with code a bit and makes it easier to read. Then functional changes. – KANJICODER Apr 15 '20 at 02:29
2

My stance is that this is an editor problem: While we use fancy tools to look at web pages and when writing texts in a word processor, our code editors are still stuck in the ASCII ages. They are as dumb as we can make them and then, we try to overcome the limitations of the editor by writing fancy code formatters.

The root cause is that your compiler can't ignore formatting statements in the code which say "hey, this is a table" and that IDEs can't create a visually pleasing representation of the source code on the fly (i.e. without actually changing one byte of the code).

One solution would be to use tabs but our editors can't automatically align tabs in consecutive rows (which would make so many thing so much more easy). And to add injury to insult, if you mess with the tab width (basically anything != 8), you can read your source code but no code from anyone else, say, the example code which comes with the libraries you use. And lastly, our diff tools have no option "ignore whitespace except when it counts" and the compilers can't produce diffs, either.

Eclipse can at least format assignments in a tabular manner which will make big sets of global constants much more readable. But that's just a drop of water in the desert.

Aaron Digulla
  • 321,842
  • 108
  • 597
  • 820
  • It's a nice idea, but I'm not sure it's achievable without ending up with source code that can only be edited in one particular environment. I think the lowest-common-denominator nature of ASCII is a big advantage. – Ned Feb 18 '09 at 11:02
  • I think it is time to move one level up. If we had decent XML editors (think next gen. HTML or close to what a word processor can do today), a lot of things would suddenly become possible: Code filtering and easy generation, nice display, etc. – Aaron Digulla Feb 18 '09 at 12:58
1

If you're planning to use an automated code standard validation (i.e. CheckStyle, ReShaper or anything like that) those extra spaces will make it quite difficult to write and enforce the rules

Ilya Kochetov
  • 17,988
  • 6
  • 44
  • 60
1

You can set your diff tool to ignore whitespace (GNU diff: -w). This way, your diffs will skip those lines and only show the real changes. Very handy!

Agoston Horvath
  • 238
  • 2
  • 8
  • 1
    That is not always possible for diff tool used in version control tool. And that means this diff tool knows about spaces in String (which must not be ignored). My old Winmerge 2.6.14 will consider (with that setting) that "" and " " are identical... Java compiler will disagree ;) – VonC Sep 19 '08 at 15:00
0

We had a similar issue with diffs at multiple contracts... We found that tabs worked best for everyone. Set your editor to maintain tabs and every developer can choose his own tab length as well.

Example: I like 2 space tabs to code is very compact on the left, but the default is 4, so although it looks very different as far as indents, etc. go on our various screens, the diffs are identical and doesn't cause issues with source control.

Nick Craver
  • 623,446
  • 136
  • 1,297
  • 1,155
  • I'm not convinced it's possible to do this sort of lining up with tabs. At least, not if you want the code to still line up with different tab-widths. – Ned Sep 19 '08 at 14:03
  • Oh, also on closer inspection, I realised I was wrong about the guidelines not pronouncing on Tabs vs. Spaces: It pretty much says never use hard tabs :) – Ned Sep 19 '08 at 14:16
0

I like the first and last, but not the middle so much.

Joel Coehoorn
  • 399,467
  • 113
  • 570
  • 794
-2

This is PRECISELY the reason the good Lord gave as Tabs -- adding a character in the middle of the line doesn't screw up alignment.

James Curran
  • 101,701
  • 37
  • 181
  • 258
  • 1
    On the other hand, it is highly recommended to use spaces and not tabs. – Eli Bendersky Sep 19 '08 at 14:15
  • I tend to agree, in so far as it's difficult get hard tab indenting right manually, and I haven't come across an editor that always gets it right. However, this is a pretty contentious point. – Ned Sep 19 '08 at 14:20
  • Tabs are nice, but there is 1/8 or 2/8 chance (assuming tab size of 8) of diff distorting the columnization (thinking of `git diff` here, although it would be possible to create a custom diff command that replaces the tabs intelligently with spaces as a post-filtering step after generating the diff as usual). – FooF May 17 '12 at 10:17