Wednesday 21 June 2017

Development : Coding Style Clash

I've spoken on these pages before, and even shown, that I generally code to a standard, one of the rules I have it NOT to use Hungarian notation, but to use a notation telling me the scope of a variable, and then give it a meaningful name.

Now, over the last few weeks I've been involved in a new piece of coding with a group of like-minded individuals.  And what getter way to explain this oxymoron than through our coding standards.

Now, I like to use "m_" for member, lots of people can accept this, I like "p_" for parameters, a lesser few reject this than you'd think and some people even quite like this as they suddenly get to reuse their useful meaningful names, and in languages like Python they suddenly get to distinguish between locals, globals and parameters really easily... But then the controvertial one, the one which causes me most angst.

Locals being represented with "l_"... You might say the lower case "L" is asking for trouble, and I would agree, but I'm sticking with "l_" locals, and so did lots of other folks, however... I said we're a group of like-minded individuals... So not a group at all.

And so locals caused a bit of a flare up, because another chap had simultaneously, and without any prompting of mine, also come to the conclusion to use this same naming convension, however with one difference... He didn't call locals locals, he called locals instances, and so he used "i_".

Yes, that was it, the whole difference... "l_" to "i_"....

I could live with this, and when coming to look at his code I could read it perfectly well, and I was more than able to leave it alone, he however, could not, and indeed would not, leave mine alone.  He went so far as to write a script which line by line, looked for "l_" and replaced it with "i_" whenever he pulled from the remote branches.

He then proceeded to commit this massive change, the result... His one liner alteration was lost in a swamp of about 35,000 changes in his check-in.  Making his contributions near impossible to track.  I asked him to keep this just to his branches, and to stick to the "l_" for the master or pushes he tagged for merging, but he simply would not listen, he changed every "l_" to "i_".

Trivial things like:

std::mutex m_TheLock;

void Somefunction()
{
    std::lock_guard<std::mutex> l_LocalLock(m_TheLock);
}

Became:

std::mutex m_TheLock;

void Somefunction()
{
    std::lock_guard<std::mutex> i_LocalLock(m_TheLock);
}

And the change logs got longer and longer, the tracking of bugs became harder and harder, until I decided to pull my support of the project, and this is important, not because it looked like a hissy-fit on my part.

But it demonstrated to me something fundamental about the project, I happily bent to others, I let them name and title things, I didn't fix their code, only stuck to my own, I published my coding standard and people started to adopt it within the group, but I never forced it upon them.  Yet this one chap wanted to import and force his will through this tiny insignificant thing.

So, always, no matter your personal feeling, stay flexible, don't be that guy who insists on something for zero gain (or in this case much loss).

[Foot note: If your project/group/team has a manager, a leader, and is not a collective as the above, please disregard this, have a coding standard, make it clear, efficient and update it regularly, but have ONE person enforce it, don't let two tribes go to war over "i_"].

No comments:

Post a Comment