Some things in life stink. Literally. Just ask any parent with a small child who is still in diapers. When something stinks, it’s natures way of telling us that either something needs further investigation (like a stinky diaper) or it’s a sign that we should stay far, far away (like a rotting corpse).
You may have heard someone say that particular code doesn’t pass the “sniff test”, or that the code “stinks”. To the inexperienced, it can be difficult to detect this. What are some indicators that code stinks?
Variables that are declared, but never used.
If it’s there but not needed, why is it there?
Poorly named functions or variables. If you ever see something like “someFunc() someFunc2() and someFunc3()”, that’s not a good sign. It’s a sign that code was either written in haste or not properly cleaned up after the fact.
Large swaths of commented out code. This indicates that code isn’t being properly cleaned up. If you need to do something drastically different, do it in a different source control branch.
No code comments at all. I recently heard of a rather controlling developer that insisted that there be no comments in code at all. While I believe that code should, to a certain extent, be self-documenting, this is an extreme.
Lots of “to-do” comments. This indicates that things aren’t done. This is probably fine if you are a single developer working on a single project, but multiple developers working in the same code should probably be using something better to track what still needs to be d one.
Functions with “or” in the name. (e.g.: GetCountryOrState()). This is a sign that things should be broken down into simpler steps. For example, it could be split into a GetLocation() function which could then call GetCountry() or GetState().
Constant variables that aren’t explained. There’s nothing quite like wondering what is so special about the number “35.10235” without anything describing what it is, or where it came from.
Excessive amounts of string parsing. Unless you are writing something that handles a ton of string related data, and you have to do some seriously funky things to it, excessive string parsing is a bad smell. Most of the time it’s not necessary, or there’s a better way of doing things.
In Python, deepcopy(). Sometimes it is a necessary evil, but most of the time it isn’t. If someone is using deepcopy(), especially if they use it a lot, that’s worrisome.
Not using stored procedures when you could be. If you are ever working with a database, you’d better be using stored procedures. I’ve seen my share of horrible things, including this. Stored procedures won’t protect you from everything, but if you are still open to SQL injection attacks because you aren’t using them, then shame on you. One particular application I worked on had SQL queries happening in button even handlers. The original developer never accounted for people like Mr. O’Conner, who happens to have a single quote in his name. A malicious user of an application like that could have easily destroyed the entire app’s database with a single, well placed string. It’s 2017 – there’s no reason why anyone should be combining strings to make queries when stored procedures are an option.
I’m sure there’s lots of other code smells, but these are the ones that stand out to me.