21

I'm currently working at a place that may be looking at forcing developers to use an automated code formatter on version control check-in. I'm looking for developers opinions on the advantages and disadvantages of doing this ... how you think it would help or hinder developers. My specific case involves Java/JSPs, but I think the question could apply to any language.

3

9 Answers 9

24

I think it's very important to do this. Here's why:

  • It makes your source control diffs show just actual code changes, and all but eliminates "diff noise" due to whitespace and other insignificant formatting choices
  • It makes all code more similar, so that devs are more comfortable pairing and sharing the code bases

If you do it, I would recommend everyone check all code in, then one person does a reformat over the whole code base, then checks it all back in so there's one "giant" change set for formatting (that everyone can ignore), but after that, all diffs are real code diffs.

If you do it bit by bit, you'll be mixing real code changes with formatting changes and things will get unnecessarily messy in change land.

10
  • 1
    Biting the bullet on a global change is really the best way, I agree; get it done and no one has to worry about it ever again. Commented Jul 12, 2011 at 15:37
  • ... until you change your style convention.
    – Nerdfest
    Commented Jul 13, 2011 at 11:24
  • 1
    @Nerdfest Then you apply your new convention on all your project in a single commit. That's no big deal.
    – gizmo
    Commented Jul 14, 2011 at 14:10
  • 2
    Your "diff" tool kind of sucks if it can't handle whitespace changes properly. Commented Jul 14, 2011 at 14:33
  • 3
    There are always going to be exceptions where a slightly different format is likely cleaner than the prescribed style. Auto-converting can thus often hide the intent of certain code blocks, which is effectively just as good as adding a defect to the code. Commented Jul 14, 2011 at 14:42
11

I'm going to toss my own answer here as people only seem to be adding advantages. What I see as disadvantages are:

  • Eliminates the ability to do 'better' than the auto-formatter ... it will undo your cleaner formatting. An example of this would be column-based parameter declarations, lists of discrete additions of objects, etc.
  • Creates a resistance to changing style conventions as these will now create large misleading diff changes.
  • Removes the ability to do any 'special-case' formatting where an alternate format would make code more readable.
  • It ties you into using IDEs which support exactly the reformatting features you need. In another IDE is missing one of the options you need, it will cause at least some problems.
  • It becomes problematic sharing a writable code repository with an external group unless they use exactly the same format conventions as your group (They usually will, but not always).
  • There are always going to be exceptions where a slightly different format is likely cleaner than the prescribed style. Auto-converting can thus often hide the intent of certain code blocks, which is effectively just as good as adding a defect to the code.

Simply put, a non-automated set of conventions sets minimum style/readability requirements, where automated conventions set a minimum and a maximum.

I remember looking at VB (version 5 maybe) and finding one of the most annoying things about it was that it would forcibly reformat my code and remove things above and beyond its basic formatting.

4
  • One formating convention over another rarely gives any benefit if it comes at the expense of consistency. You get use to it. Follow Bohemian's advice and create a grace period to help determine the formating styles and then stick with them. Changing the format should not be taken lightly anyway.
    – JeffO
    Commented Jul 12, 2011 at 14:24
  • 3
    @Jeff, I don't believe he's advocating inconsistent code formatting, but rather consistent code formatting that is difficult to automate. For example, many coding styles specify aligning columns in an aesthetic manner when you have several lines of related data together. This greatly enhances readability, but is very difficult to automate definitions of "aesthetic" or "related." That's the reason some code formatters will allow manual formatting to override under certain circumstances. Commented Jul 12, 2011 at 15:07
  • 1
    Its far better to have consistent format 99.9% of the time and put up with the odd bit you personally don't like, than to put up with an undisciplined mismash. I probably depends how disciplined the team is where the balance lies. If you stick to the established norms for a language, all decent editors/IDEs will be capable of formating that way. If you insist on moving from the norms, you will have trouble.
    – mattnz
    Commented Jul 14, 2011 at 23:26
  • Thanks for presenting the other side. I do think that your points 1, 3, and 6 boil down to the same thing, and can be solved by occasional use of //@formatter:off.
    – K--
    Commented Jun 30, 2022 at 14:12
4

I find that forced code formatting is great. It allows a developer to traverse the entire corpus of code without having their eyes bounce everywhere. Also having this standard in place helps novice developers break bad habits.

3

Primary disadvantage is losing custom formatting where it really matters.

Imagine a typical sanity check if() that will fail if any of the specific conditions is present but not fulfilled...

  if(
      (user.id == TEST_ID)
    ||(
         (user.id == UserID)
       &&( 
             ( user.type == HUMAN_USER && user.name.size() >= MIN_NAME )
           ||( user.type == EMULATION && input.source != SOURCE_INTERNAL ))
       && ( user.email == NULL || emailValidator.isValid(user.email))
       && ( (user.phone == NULL) == (user.type == EMULATION) )

       // several more lines like this.)
    ){ /* handle results */ }

This is readable thanks to reasonable indenting following the logical structure of the conditions.

Now your automated tool has no clue about logical separation of different conditions into related lines. It sees no reason why each clump 3-4 conditions in one line and split the next condition in half. Or it will split it, one comparison expression per line. It may even look prettier on screen but the logic will be lost.

3
  • 1
    This is mess, my positron brain just melted. So much inconsistency with whitespace around ( and ). That's exactly why we need machines to format the code. And you could always put single line comment before/after (depends on your convention) to force condition grouping. It would also be helpful to explain reader the business rules behind this conditions - right in this single line comments. Commented Aug 29, 2019 at 10:11
  • @ŠtefanOravec: Run this through an autoformatter and apply these comments. See if it's more readable. Test user - always. Human users with valid usernames. Emulated users - only external sources. Email, if present, must be valid. Human users must have a phone number; emulated - must not. See how your single-line comments align with the autoformatted code.
    – SF.
    Commented Aug 29, 2019 at 11:49
  • This seems like a great example of why you should use automatters. An autoformatter might give an unreasonable result here because this is a massively complex, incomprehensible condition that needs to be broken up. It exhibits very idiosyncratic, personal styling choices in indentation and spacing that may look good to its author, but not to most. If you broke these conditions out to one or more functions, the formatter would have no problem with it. In other words, autoformatters help hold programmers accountable--if the code looks bad on auto-format, in most cases, it probably is bad.
    – ggorlen
    Commented May 22, 2023 at 22:59
2

In my experience, it's a good thing. Without one, code compares often show a mess of whitespace formatting, and may hide actual code changes. In my experience, messing with someone's formatting isn't the sin it's made out to be, especially with the potential benefits of consistency across the team.

2

I've added an answer with disadvantages, and I'll throw in what I consider a big advantage as well.

When you use an automated code reformat on commit, it does actually opens up the possibility of personal preference variations without the usual effect of having your preferences inflicted on others. You can have your IDE format code to a common standard on commit, but display it to you in your preferred format without affecting others.

This to me is almost the Holy Grail of convention based coding ... you get the advantages of a common code format, but still allow personal preferences to be supported without conflicts.

0

It depends on your needs but some contraints are quite helpful, e.g. every if() should be followed by curly braces, since it's quite easy to get such an if wrong if you're refactoring.

Consider that case:

if( x == 0 ) 
   return foo;
//do something else
return bar;

If you now want to add some logging to the if case you might accidentially write:

if( x == 0 ) 
  log.info("x was 0");
  return foo;
//do something else
return bar;

And suddently your method always returns foo.

Edit: that's not on automatic formatting but rather style checking. Sorry if that answer was off topic as well. :)

11
  • That's more a code style thing, not a formatting thing. There are build tools for that.
    – Bohemian
    Commented Jul 12, 2011 at 12:44
  • @Bohemian, you're right, I misread the question. The build tool of our choice is checkstyle btw. :)
    – Thomas
    Commented Jul 12, 2011 at 12:46
  • Autoformatters would actually help catch this because they'll de-indent the return to match its true block, and probably add braces as well, making the error very obvious. That said, any good compiler or linter should detect the unreachable statement.
    – ggorlen
    Commented May 22, 2023 at 23:05
  • @ggorlen I'm not so sure I'd want to rely on the compiler or linter as not all cases would lead to unreachable statements. Also, while auto-formatters would help detect this by creating a code smell it would still depend on a human to actually read the code again and understand the original intent. To me having a constraint in place to force developers to explicitly state their intent, e.g. by adding curlies, is preferable to guessing or trying to spot unintended errors after the fact - even though this might add some inconvenience.
    – Thomas
    Commented May 23, 2023 at 11:54
  • Autoformatters can add those braces. Sure, one should be on the lookout for unreachable statements, but the point is that autoformatting will make logical/flow errors more obvious on sight than not, so this does seem like a genuine advantage. Compilers and linters on full blast settings are also important (probably more so, but finding bugs is a team effort).
    – ggorlen
    Commented May 23, 2023 at 15:14
0

It greatly helps to uniform the code in the company, and thanks to that you generally produce a more easily understandable and way more easily maintainable structure for your product.

0

Well, the advantages are the same as any code formatter, like standardization of the code, semantic between developers, etc. The only possible disadvantages I see is the lack of a human eye after the formatting, to add some exceptions, etc.
So I guess is better consider a IDE formatter instead of a check-in time formatter.

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