I am part of a consultant team implementing a new solution for a customer. I am responsible for the majority of code reviews on the client-side codebase (React and javascript).

I have noticed that some team members use unique coding patterns to a point that I could pick a file at random at tell who was the author from the style alone.

Example 1 (one-off inline functions)

React.createClass({
  render: function () {
    var someFunc = function () {
      ...
      return someValue;
    };
    return <div>{someFunc()}</div>
  }
});

The author argues that by assigning a meaningful name to someFunc the code will be easier to read. I believe that by inlining the function and adding a comment instead the same effect could be achieved.

Example 2 (unbound functions)

function renderSomePart(props, state) {
    return (
      <div>
        <p>{props.myProp}</p>
        <p>{state.myState}</p>
      </div>
    );
}

React.createClass({
  render: function () {
    return <div>{renderSomePart(this.props, this.state)}</div>;
  }
});

This is how we usually do it (avoids having to pass state and props):

React.createClass({
  renderSomePart: function () {
    return (
      <div>
        <p>{this.props.myProp}</p>
        <p>{this.state.myState}</p>
      </div>
    );
  },
  render: function () {
    return <div>{this.renderSomePart()}</div>;
  }
});

While these coding patterns are technically correct they are not consistent with the rest of the codebase nor with the style and patterns that Facebook (the author of React) hints at in tutorials and examples.

We need to keep a fast pace in order to deliver on time and I don't want to burden the team unnecessarily. At the same time we need to be on a reasonable quality level.

I am trying to imagine myself as the customers' maintenance developer faced with inconsistencies like these (every component might require you to understand another way of doing the same thing).

Question:

What is the value as perceived by the customer and its maintenance developers of a consistent code base vs. allowing inconsistencies like these to remain and potentially spread?

  • 50
    What is the value of a consistent orthography? A permanent, constant speed-up in reading and writing texts. What is the value of consistent code style? A permanent, consistent speed-up in reading and writing code. What else could you want? – Kilian Foth Jul 7 '15 at 20:17
  • 9
    When you read things you get used to patterns in what you're reading, and come to anticipate how to read future things. If there is no consistency one can't anticipate patterns and more interpretation of code needs to be done, slowing the reader down. – mcraen Jul 7 '15 at 20:27
  • 1
    Joel has some good ideas on that topic. In short, a good coding standard makes bugs easy to see. joelonsoftware.com/articles/Wrong.html – user186406 Jul 8 '15 at 1:46
  • 13
    I want to add that named functions are almost always preferred to anonymous functions in JavaScript. When debugging this helps a ton to determine where you are in the stack. – Mike McMahon Jul 8 '15 at 4:51
  • 1
    @yoniLavi you should ask that on meta. – RubberDuck Jul 8 '15 at 23:31
up vote 48 down vote accepted

Code Transfer Advantage

Following patterns provided by a library, React in your case, means that the product you deliver will be easily picked up and maintained by other developers who are also familiar with React.

Potential Backward Compatibility Issues

Some libraries would have a new major version out, and backward compatibility might be compromised if your patterns are significantly different, thus slowing/halting your future upgrade. I am not sure how React would deal with new releases, but I have seen this happen before.

New Members on The Team Start Being Productive Quicker

If you follow what is provided by the author, you are more likely hiring talented developers using your framework and start them off quicker with your system rather than teaching new patterns.

Potential Undiscovered Issues

There might be issues in the future that you haven't discovered yet with your approach, that are solved by the author's approach.

That being said, innovation is always a risk, if you strongly feel that your approach is better and it works for your team, go for it!

  • Thank you for these excellent points. We are generally following the patterns provided by the library. The examples I've given (found in code reviews) deviate. I've been worried that the customer might reject and require us to redo a part of our delivery because it didn't "look and feel like React". Now I know why they would do that. – Andreas Warberg Jul 7 '15 at 20:36
  • 2
    +1 "rather than teaching new patterns" - procedural training is a very large time sink with new hires. Always strive to minimize the time cost by piggybacking well known patterns – Gusdor Jul 8 '15 at 7:19
  • 1
    @Alexus: Regarding backwards compatibility, React is still not at version 1.0. The current version, 0.13, did break compatibility in some pretty drastic ways, though whether these failures will happen depends on what mechanism is used to call React components. Having very consistent rules for how to call React may not prevent React upgrades from breaking code, but fixing consistent code is easier, faster, and more reliable. Your concerns about backward compatibility issues are definitely justified in the case of React. E.g., see my comment on stackoverflow.com/a/20913637/18192 – Brian Jul 8 '15 at 14:12

Inconsistencies make you stop and think why, which and where:

  • When you read part of the code and see that it uses a different style from the rest of the code, it makes you wonder - why is this particular part different? Does it have a special reason that I need to be aware of? This is dangerous, because if there really is a reason for that part to be different you need to be aware of it or else you might create some nasty bugs. So instead of thinking on the original problem that made you go touch that code you now waste time thinking on why it's different, and you can't figure that out so you go to the original author to ask them, wasting their time too, and even worse - in the process you both loose the mental model of the problems you were working on.

    Multiply by every other developer in the team that needs to touch/read that code.

  • When you need to add to a code that use multiple styles, you need to stop and decide which style to use for your addition. You have to make enough decisions that matter - it's a waste of time to waste time thinking about decisions that don't matter.

  • When the style is consistent, it's easy to navigate around the code, because the consistency helps you quickly find where stuff is located. With inconsistent style even grepping becomes harder because you don't know what style the thing you are looking for is using.

  • 6
    Great, fanstastic insight. Why do some developers seem to just 'get' this, and others fight against it? – Dogweather Jul 8 '15 at 0:34
  • 2
    Suspect because a consistent style being proposed now is deeply inconsistent with what they have worked on in previous projects. Especially for those with a substantial amount of experience in different environments. – Kickstart Jul 9 '15 at 11:17

The code may be well written yet not perfectly consistent, so some clients won't care. When things start to go bad, do you want to give them another knock against you? If I hired a company to work on a multi-developer project and they didn't indicate to me that they have a coding standard and follow it, I would be skeptical.

We need to keep a fast pace in order to deliver on time and I don't want to burden the team unnecessarily.

As long as the coding standards aren't too crazy, it shouldn't be that hard for everyone to get on board. Projects get stalled because people don't know what code to write or not write and not because of typing and formatting. You'll know you've gone to far when it takes a disproportionate amount of time to adhere to. Hopefully, this isn't your first rodeo.

Conduct your own test All you need to do is switch assignments midway from one dev to the other and have them finish someone else's file. Do they spend time completely reformatting things to their version? Is it too difficult to follow? It will get worse for your client's maintenance team.

I have had good luck treating code styles just like I treat writing styles in natural English. There's a huge range of styles from conversational English, which mimics the way we speak in every day conversation, to formal English, which is often heavy and stilted by always very precise in its meaning.

Consider what you would like the final product to look like in that sense. Some situations are very supportive of using whatever structure is best at the time. Others require a uniform cadence and structure. This is especially true if your product is best though of as though it was written in formal English. Colloquialisms may help in that case, but they tear at the overarching feel of the product.

In general, the more consistent your coding standard is, the less effort a developer must take to call their attention to something interesting. If you support great diversity in coding standards, developers must be louder when announcing that they are doing something that is unusual or unique. This attempt to express what a developer considers to be important can often overshadow the dynamic range of the code itself. When this happens, it is easy to have bugs slip through because its simply too hard to see them.

On the flip side of that argument, the looser your coding standards are, the more freedom developers have to adapt their syntax to the problem. In ironic contrast with the pro-coding standards argument, too much standardization can lead to stilted code structure, which can also make it easy to have bugs slip through.

What you are looking for is your own team's happy medium.

As several others have pointed out, when the maintenance developers have to go behind you, a section of code in a different style is going to cause them to stop and try to figure out what is special about that section. There's also the very real risk of the inconsistent style getting propagated to other sections, resulting in a huge mess later.

I get the impression that, deep down, you already know the answer to this question, and you wouldn't even be facing it if it weren't for this:

We need to keep a fast pace in order to deliver on time and I don't want to burden the team unnecessarily.

This always seems to be the universal trade-off... doing it quick vs doing it right. Only you can evaluate the consequences of sacrificing good coding practices on the alter of customer deadlines. However, I have to agree with JeffO when he observes that following a (possibly unusual or counter-intuitive) coding style shouldn't slow down your team so drastically that the deadlines slip significantly.

Although I've known about the concept for years, I only recently learned the term "technical debt." You really need to consider the technical debt that will ultimately have to be paid if you don't follow a disciplined style now.

Unfortunately, as eBusiness stated, unless your customer is particularly technically savvy or is going to be maintaining the code themselves afterward, it's hard for them to appreciate the value of consistent coding standards. However, any reasonable businessman should be able to grasp the concept that "a bit of preventative maintenance now" (in the form of good coding) "will help avoid significant repair costs later" (in the form of wasted developer time later cleaning up the mess).

  • The customer is technically savvy and will most likely take over the maintenance and further development of the solution at some point. The Q&A effort so far has focused on the visual layout and not so much on the functionality. Once functionality bugs start coming in we will get a feeling for what maintenance is like. I think code reviews would be faster with more consistency (reduce surprising new ways of doing the same thing). The customer obviously wants the maximum consistency (but maybe not pay extra for it). – Andreas Warberg Jul 9 '15 at 16:09

The topvoted answers here repeat the easily agreeable theoretical best practices detailing how we would all like our codebases to be. But real code somehow never looks like that. If you try to make that perfect codebase, you will almost inevitably fail. That is not to say that we shouldn't try to do better, but there has to be a limit for how far from the realistically achievable we set our goals.

Too much focus on minor stuff has the risk of losing focus on more important issues, like how to solve the job in the overall most efficient manner.

I wouldn't refer to you first example as style, style is the choices where there in no clear right or wrong, in this case the extra function is code bloat with no upside. It is only 2 extra lines, still easy to read, and easy to fix, so this example itself is no big issue.

The much bigger issue is the convoluted code bloat. Wrapper functions, class hierarchies, and all sorts of other constructs, where the surrounding code is complex enough that it isn't obvious that they serve no real purpose. If there is obvious bloat in the code, there is probably a lot more non-obvious bloat. It is a lot harder to do something about, and harder to spot, but also a far bigger issue.

About customers

Customers tend to focus on getting a working product that solve their needs. Even if you have one of the few customers that do worry about code quality, it is going to be a secondary priority, and their idea of code quality might not be perfectly consistent with yours. The customer company may have its own programmers, but it is still management that will decide whether or not you did a good job, and management most likely doesn't even know what code quality means.

  • When reviewing code I look for several things that I believe are definitely important such as direct DOM manipulation, unlocalized user-facing strings, opportunities for reuse (an existing component, helper, already-loaded 3rd party library etc.), incompatible or improper changes to shared code, deviation from the customer- and team conventions. The value of consistency of style and coding patterns was not clear to me so I was unsure how to respond to those in code reviews. – Andreas Warberg Jul 9 '15 at 15:34

It's very much about the legibility of the diffs. If code style thrashes around, the diffs hide changes with no semantic meaning to the program, and can make merges hard or impossible.

protected by gnat Jul 9 '15 at 6:55

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.