Question

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?

Was it helpful?

Solution

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!

OTHER TIPS

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.

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

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.

Licensed under: CC-BY-SA with attribution
scroll top