Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

This question already has an answer here:

By "code style" I mean 2 things:

  1. Style, eg.

    // bad
    if(foo){ ... }
    
    // good
    if (foo) { ... }
    
  2. Conventions and idiomaticity, where two ways of writing the same thing are functionally equivalent, but one is more idiomatic. eg.

    // bad
    if (fooLib.equals(a, b)) { ... }
    
    // good
    if (a == b) { ... }
    

I think it makes sense to use an auto-formatter to enforce #1 automatically. So my question is specifically about #2. I like to break things down into pros and cons, here's what I've come up with so far:

Pros:

  • Used by many large codebases (eg. Google, jQuery)
  • Helps make it a bit easier to work on new areas of the codebase
  • Helps make code more portable (this is not necessarily true)
  • Code style is automatic once you get used to it
  • Makes it easier to fast-decline pull requests

Cons:

  • Takes engineers’ and code reviewers’ time away from more important things (like developing features)
  • Code should ideally be rewritten every 2-3 years anyway, so it’s more important to focus on getting the architecture right, and achieving high test coverage
  • Adds strain to code reviews (eg. “don’t do it this way, I like this other way better”)
  • Even if I’ve been using a code style for a while, I still sometime have to pause and think about how to write a line better
  • Having an enforced, uniform code style makes it hard to experiment with potentially better styles
  • Maintaining a style guide takes a lot of incremental effort
  • Engineers rarely read through the style guide. More often, it's cited in code reviews

And as a secondary question: we also have many smaller repositories - should the same code style be enforced there?

share|improve this question

marked as duplicate by gnat, Ampt, Bart van Ingen Schenau, GlenH7, Kilian Foth Aug 19 at 8:43

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

1  
Which language is this? The first one is barely noticeable, if some tool handle it great if not, who cares? The second one is a bit dependent on the actual language. So can't really say anything but, why bother :) –  hkon Aug 18 at 17:00
    
Use a linter or some program or tool that automatically enforces style. –  Robert Harvey Aug 18 at 17:01
    
@hkon this applies to any language. In the second example, let's say both work but only one is idiomatic. –  bcherny Aug 18 at 17:02
    
@RobertHarvey that would enforce the first type of code style, but I imagine writing a linter to enforce the second type would be very difficult, if possible at all –  bcherny Aug 18 at 17:03
1  
Most of your cons aren't really cons. Code styles shouldn't take time away from engineers or reviewers - it can be automatically set by an IDE or a pre-commit hook to change formatting. Code reviewers should be looking at well-formatted code so they can find other issues (things that can't be detected by static analysis or issues with the automated tests). Having a standard style doesn't mean someone can't propose a new style on a different project to try out and see what people think –  Thomas Owens Aug 18 at 17:15

3 Answers 3

Use a linter or some program or tool that automatically enforces style. That should alleviate the objections about it taking too much time.

Most of the objections have to do with "I'm uncomfortable with that style." It doesn't matter. Coding conventions are all about uniformity. Enforcing that uniformity will make code reviews easier by eliminating discussions about that, and allowing code reviews to focus on more substantial issues.

share|improve this answer
    
I'm not interested in the automatable part of code style (#1 above). I'm specifically asking opinions on the non-automatable part (ie. what's idiomatic, #2 above). Thanks! –  bcherny Aug 18 at 17:17
    
That's covered under "more substantial issues" in my answer, above. –  Robert Harvey Aug 18 at 18:12

The point is that you need to use an automated tool which enforces style on every commit. The applies to both examples from your question. In .NET, the first piece of code will be enforced by StyleCop, while the second one—by Code analysis (for inspiration, see CA1820).

The goal of having a style guide is to:

  • Enforce uniform style in order to make the code base easier to read and maintain by reducing the effort required by a different style than the one the reader usually uses. Example: tabs vs. spaces.

  • Discourage situations where bugs are introduced more easily. For example, by forbidding to end the line of code by whitespace, you may mitigate the risk of introducing some bugs which are moreover difficult to pinpoint:

    #!/bin/bash
    cp·-r·/hello/world·\·     ← This space at the end is annoying.
          /demo/destination/
    

In both cases, rules can and should be automated.

Once you use an automated tool, the cons from your question become irrelevant.

Takes engineers’ and code reviewers’ time away from more important things (like developing features)

If you use automatic style checker, correcting those errors at the beginning can waste some time to the authors, but with practice, it will quickly become a very easy task which takes in worst cases a few minutes.

Code should ideally be rewritten every 2-3 years anyway, so it’s more important to focus on getting the architecture right, and achieving high test coverage

There is nothing to "focus" on. You write code, you commit it, if there is a style error, you correct it and commit again. And you do that while keeping all your focus on the readability of the code, maintainability, architecture, design and reliability/testing.

Also, why do you have to rewrite code every 2-3 years? Shouldn't you be concerned by the quality of the code which requires such time-costly rewriting?

Adds strain to code reviews (eg. “don’t do it this way, I like this other way better”)

You shouldn't mention nor discuss style during code reviews.

Even if I’ve been using a code style for a while, I still sometime have to pause and think about how to write a line better

Better as opposed to compliant to the style guide? In four years using StyleCop and a few months using PEP 8, I never found myself in a situation where adopting a non-uniform style would substantially improve the readability.

Having an enforced, uniform code style makes it hard to experiment with potentially better styles

Which doesn't matter. Frankly, there is no right answer to whether you should use tabs or spaces or whether you should put { on a new line. There is nothing to experiment with. Most rules are somehow arbitrary: what counts is not which rule was chosen, but the fact that the rule was chosen and is now enforced, making the code base consistent.

Note that you shouldn't follow style rules religiously, without understanding why are they written in their current form. Some rules (tabs vs. spaces) really don't have a convincing explanation. Others have an interesting reasoning. For example, this piece of code:

var demo = [
    someElementHere,
    anotherOne,
    helloWorld,
    goodBye,
];

is better than:

var demo = [
    someElementHere,
    anotherOne,
    helloWorld,
    goodBye       ← Noticed the lack of comma?
];

Why? Because in the first case, if you add a new element to the array and commit, the commit consists of one new line. In the second piece of code, the commit will consist of one new line and one modified line. The first one is semantically more correct in a diff.

Another interesting example is code used to advertise jsHint as being better than Douglas Crockford's jsLint. The code contains a mistake which is not obvious to see by a beginner. jsLint catches this mistake and displays a error. jsHint doesn't catch it, letting you to find it yourself when one day, in production, the web app starts behaving unexpectedly.

Maintaining a style guide takes a lot of incremental effort

Don't reinvent the wheel. Most languages already have a style guide. A benefit of using an existent one is that most newcomers will already know it, whereas nobody will know yours.

Engineers rarely read through the style guide. More often, it's cited in code reviews

That's why it should be enforced during commits.

Now your main question:

Should we enforce code style in our large codebase?

Ask your team. Is code style inconsistency an issue for your team? Are at least some members constantly complaining about the style of their pairs? Do you have commits which intentionally change style, and only style? Do you have unreadable diffs because someone decided to replace all tabs by spaces or add spaces before and after each parenthesis?

  • If yes, you have to enforce code style. Otherwise, the team members are wasting their time.

    Note that this task is not easy. You want to avoid discussions about which style is better at all costs: otherwise, you'll find yourself making compromises, and finally, you'll end up with no style enforcement at all.

    If you're interested in the difficulties of introducing a style into an existent code base, ask a separate question, and I'll be happy answering it in more detail.

  • If not, don't bring the subject. Team members are happy with the current inconsistency, or at least they learnt to deal with it. Many developers are in this case: often, ones who switched jobs a lot or who used a lot of different programming languages can't care less and are able to read and understand code with any style.

    Enforcing style in this case would only slow things down. The team would be forced to walk through the entire code base. The bigger is the code base, the harder is to fix the style. If the code is not tested enough, there is also a risk of introducing regressions.

share|improve this answer
    
To clarify, I propose there are 2 kinds of code style: (1) Code style that is automatable (spacing, tabs, etc.), and (2) Code style that is not automatable (ie. What is idiomatic, “this line can be better written like this”). Since the cost of enforcing #1 is near-zero (if we use automated tooling), I’m asking specifically about the 2nd kind of code style. –  bcherny Aug 18 at 17:52
    
@eighttrackmind: can you please give an example? Both examples from your question can easily be checked with an automated checker. –  MainMa Aug 18 at 17:59
    
I don't want to get lost in specific examples. The point is that there exists a class of code review comment that is somewhere between simple code style (eg. spacing) and architecture-level comments. Here's another trivial example, creating a new array of 100 "foo"s in javascript: gist.github.com/eighttrackmind/8382e78b0712f628889b –  bcherny Aug 18 at 19:04
    
@eighttrackmind: examples are important; you claim that some style rules cannot be automated, but cannot give an illustration. Both examples from your question are easily automated. As for 100 "foo"s, jsLint would invalidate option 2 (as well as option 1, but that's more subtle); then, enforcing functional style in an automated tool won't be difficult either. –  MainMa Aug 18 at 19:14
    
@eighttrackmind: maybe you believe that linters are no more complicated than grep. In reality, they are much more than that, and in order to be convinced, look at the rules of jsLint of Code analysis (including the link to CA1820 in my answer). –  MainMa Aug 18 at 19:15

I think the code style works the other way -- it helps reviewer understand the code easier, save their time, and helps maintenance in the future.

Also, reviewers shouldn't force others to use a code style, but they should point things out if that gonna cause some trouble (especially seniors).

Automatic style tool should be ok but also should be optional -- as some developer might not like to style their code for some reasons.

share|improve this answer
3  
I don't care if a developer likes a particular style or not. The group needs to have a consistent style. –  Snowman Aug 18 at 18:04

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