23

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?

1
  • First and third examples are well and good, but I think the second may call for it to be separated into lines. Nov 5 '10 at 5:27

14 Answers 14

29

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.

6
  • 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? Jul 6 '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 8: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 7:39
24

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.

1
  • 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
19

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.

4
  • 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 8: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 0: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 0:08
  • How long does it take to invoke a pretty-printer?
    – jwg
    Aug 1 '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.

10

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.

4
  • 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
  • 7
    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. 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. 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
7

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.

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.

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

1
  • 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. Apr 15 '20 at 2: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.

2
  • 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. 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

2
  • Since no one seems to know we have a standard, this seems fairly unlikely in our case :)
    – Ned
    Sep 19 '08 at 14:10
  • 1
    this is exactly why you need automated code standard validation. start using it Sep 19 '08 at 16:07
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!

1
  • 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.

2
  • 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.

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

3
  • 1
    On the other hand, it is highly recommended to use spaces and not tabs. 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

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