Thursday, August 16, 2018

You're doing it wrong: Coding Styles

There has been lots and lots written about coding styles, and I'm writing still more. My basic premise is the same as most of the other articles - everyone else is wrong, so listen to me.

I see the same basic issues pop up over and over again in coding guidelines. The most egregious, or at least the ones that come to mind as I write this, follow:

  • Detailed Guidelines that cover every conceivable syntax case: Here's the first rule. If your coding guidelines take up more than a single page of normally written text - including properly spaced examples - you've got too many rules. Remember that the purpose of software development is to create code, and the purpose of having coding guidelines is to reduce bugs. If your guideline can't reasonably be defended to reduce common bugs, it isn't needed. This is subjective, but this very basic guideline can trim the size of many lists. If you have too many, people won't even remember them, let alone use them.
  • Getting picky over the little things: It's very common to enforce things like whitespace. This tends to be pedantic after a point. In truth, I approve of little things like aligning lists of initializers or cases. (Some people are ridiculously against this, their defense is they don't want to read code in a code review. Use 'ignore whitespace' then. Aligning the code leads to a faster code review because you can just scan the list, instead of searching each line for the important parts. I'll complain about code reviews again someday.) Ultimately, the purpose of software development is to create code, and the purpose of having coding guidelines is to reduce bugs. It's rare that tabs versus spaces break the code.
  • Created by a secret cabal: it's pretty common that coding guidelines are developed by the senior developers and everyone gets to follow them. But unless you're a mega-corp with a company-wide policy and you can't change the rules or your job, the guidelines should be developed by the entire team - even that kid who just finished college and it's his first day and he hasn't even done orientation or dealt with IT yet. There are two very important reasons - it provides a feeling of contribution, and you all have to deal with them - so you might as well agree on them. Be diplomatic but firm about unnecessary rules - but if someone is very persistent and nobody else really cares - maybe consider adopting the rule, even if it is tabs instead of spaces. In truth, the purpose of software development is to create code, and the purpose of having coding guidelines is to reduce bugs. If people don't like the rules, they won't follow them or enforce them, and you've just wasted everyone's time.
  • Not well communicated: I think just about every rant in every field can include this bullet point. You can't just post a note on a semi-private IM chat channel and say you've communicated the guidelines to the group. Communication means making sure EVERYONE has had their say. There are two important parts there - "EVERYONE", and "making sure". Push notifications might cover everyone (might), but if you didn't make sure, you didn't communicate. You broadcast. There's a difference. In the end, the purpose of software development is to create code, and the purpose of having coding guidelines is to reduce bugs. If people don't even know about the guidelines, it's just wasted time (not to mention that means they didn't get their input as in the previous point).
  • Based on the latest trends: also known as "hype-driven development", trends should never be enforced. Most of the time they don't last, and the rest of the time only one or two of your developers know what they are and what they mean anyway. This means that coding the latest cool trick or using some amazing style that you found on a web page will only serve to confuse everyone else. Do note, the purpose of software development is to create code, and the purpose of having coding guidelines is to reduce bugs. If people don't even understand the intent of the whizz-bang feature, they can't effectively use it.
So what is important, then? Coding guidelines should aim to make the code readable. Coding guidelines should include comments - hell yes they should. Comments describe what the code is supposed to do and why it's doing it the way that it is (comments shouldn't describe what the code is doing unless it's really unclear). Coding guidelines should discourage 'tricks' that make the code hard to read (even if it works and is neato-keen) and encourage styles that let the compiler help find bugs. But most importantly, coding guidelines should exist to help the programmer get their job done. 

A secondary goal is to help the code reviewer get the code reviewed, because of course you are using code reviewers who are enforcing the guidelines, right? The code reviewer has their own code to get written, and needs to get through the review as quickly as possible. Basic syntax rules based on common errors make it easy to check if a code should be rejected without having to analyze whether it's actually faulty, which is a huge time saver. At the same time, reviewers should be generous when considering non-bug-causing issues, like whitespace alignment. A programmer who is running through his code fixing whitespace and unaligning lists and alphabetizing includes is not writing new code or testing existing code, which one would expect, is what we'd prefer they were doing. At least if I was paying them, that's what I'd prefer. In addition, after doing all those meaningless cleanups, they then need to repeat all the testing they previously did. Whitespace cleanup costs real money, and software developers are generally not cheap. Ask yourself if changing that tab to a space is really worth $500 or more (time to checkout, fix, build, re-test, commit, restart the code review, reviewers to come check it again...), or if you can just get it next time.



No comments:

Post a Comment