T-SQL Tuesday #151 – Coding Standards

It’s another T-SQL Tuesday! This month, Mala Mahadevan (t | b) is hosting. Her question for us this month is:

What are the T-SQL coding rules that are appropriate for where you work and what you do? 

When I think of what my coding standards are, I tend to think of it as how would I review scripts for a pull request (PR). I think my past coworkers can attest that I can get quite picky when I look at code. So if I were your reviewer, what sort of things would I comment on?

• Readability – Can I read your code? I feel like this is a hard one as the sole basis for rejecting a code review. I may make a comment, especially if it means that a bug could be lurking somewhere.

• This also brings the debate of “Team Spaces” or “Team Tab”. Because of different editors, different font types and sizes, diff tool compares, etc., I could make an argument that tabs would create more consistency because a tab is a tab is a tab in any editor. But if you like spaces, as long as everyone agrees that we’re using X number of spaces, regardless of how it looks in the editor, and every developer goes into their settings to set it to be the same, I don’t really care. It’s a minor annoyance that I won’t ding you on. As long as it’s easy for others to read it, that matters.

• Do the object names make sense and, more importantly, are they following the company’s naming convention? Notice I say the “company’s naming convention.” Do I have my own preference? Of course I do. I feel like there’s a really good argument for using singular table names because you have to talk about a single record in that table and linguistically, it’s much easier if the table name is singular. Is it worth throwing a fit if the company I work for doesn’t use singular names? Not really. If your table names or column names don’t make sense, I’ll talk to you about that. But I do hate having to troubleshoot performance where the index is named IX_table_3_5_6_14_2 so I then have to go look up the table to figure out what the index is covering. (This example would be columns 3, 5, 6, 14, and 2 – whatever they may be.) I say often, I like looking at the name of an object – any object – and knowing exactly what it means. More importantly, does that index name follow our convention? No? Reject that PR!

• Are there comments? If not, would that help us understand the code? If there are comments and spaces for the comments, are they being updated? I’m a huge fan of commenting early and often. I add comments to the beginning of procs. I’ve been known to add comments to table scripts to indicate column definitions. And I’ll add those comments when I check code into source control. I don’t always trust that I remember why I did something so why should I expect you to figure it out? And if you haven’t guessed, my comments are quite verbose. For those who just add comments to the check-ins to source control (yes, I know – I’ll wait for you to finish laughing since who does that, other than me?), it assumes that those who are troubleshooting code in the field have access to it. Same goes for those who just use the ticket number in the comments without anything more. If you have a space in the script to say what the changes are, add them. If you’re using nonstandard\non best practice code for a particular reason, let people know why. I would add your initials\name to those comments in case of questions, especially when multiple people have eyes on the code.

• Are you following best practices when it comes to the code? Is there anything that may cause bugs down the road? Is anything missing that should be there? This is a pretty vague answer because it gets too broad for a single bullet point. But it does mean that there’s a lot to cover when it comes to reviewing scripts to make sure standards are being met. Not only that, are there any obvious bugs in the code – like someone forgot to add the columns for an ON clause for a JOIN.

Will not doing these cause me to fail your PR? This is why I say it depends. I’m not expecting everyone to write their code like I would but if it meets the basic requirements of what’s been agreed upon by the team\company\architects\whoever is in charge of these things, I’ll likely approve it. If your code alone is sloppy, I may still approve as long as it’s still reasonably readable. If your object name isn’t great but at least it conforms to naming standards, probably not. But if you’re not naming your constraints properly, I will make you change it. If you forgot to comment but we have big sections at the top of proc to put them, I’ll suggest that you add it in. If there are mistakes in the code or you’re using code that will cause problems down the road, I will reject the PR and send it back to be fixed. If it’s not great but we can get away with it for now, especially if we’re under pressure to get it out, I’ll say why it’s not great but why we can get away with it for now but add that it should be fixed later. Unfortunately, I’m a realist and that’s just the reality sometimes.

The biggest thing with all of this is that everyone is consistent with their code. And everyone buys in to make sure that everyone else is consistent. You can’t have one person on the team who is making sure company standards are being followed. They then get burdened with the “script police” job of making sure that all the i’s are dotted and T’s are crossed. And it can prevent that person from doing other work because it takes too much of their time. And it can prevent you from doing your work because you now have a bug to fix. But when everyone is making sure that they’re using these standards and that they’re making sure that others are using these standards, it’s easier to enforce.

I’m enjoying reading everyone else’s take on this and look forward to reading this rest. Thanks for the topic, Mala! Happy Coding, y’all!