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?
