Code smells 👃💩🤢

When you have to maintain other people's code, little things can give you a bad feeling. They aren't disasters, but they don't give you confidence in your predecessors.

Smelly database tables

Looking for the table of Things. I was expecting a database called SensibleName, and there it was. Luckily a colleague told me to ignore that and look in SensibleName_Acronym2.

• Smell 1: this is a production system, and the obvious database isn't the one you want.
• Smell 2: what's Acronym doing in there?
• Smell 3: there is no SensibleName_Acronym or SensibleName_Acronym1 so what's the 2 for?

Never mind. I scan down the list of tables for the Thing table. It's OpenThing2.

• Smell 1: some tables in the list are named in CamelCase, some in snake_case, and some in CamelSnake_case, a convention so peculiar that I've just had to make up a term for it.
• Smell 2: there's no OpenThing or OpenThing1, so what's the 2 for? I notice a pattern here.

You could argue that "Open" is a smell because it's a status of Thing, and there are other StatusThing tables, but the fields on other StatusThing tables differ, so I'll let that go.

Smelly source code

You can't ignore a checked exception¹ but you can do this:

   catch(Whatever w){
      ;
   }

Presumably the original dev was annoyed at having to use try/catch, so they just swallowed the exception they didn't want to deal with. Then I'll bet that they got a warning for an empty catch block, so they put an empty statement in to make the warning go away.

The same code base also had catch blocks like this:

   catch(Whatever w){
      log("Got an exception");
   }

It's saying "Something went wrong, but I'm not telling you what." If it's important, log some details (at least). If it's not important, why log a teaser? What it actually does is really the same as the previous example, but more annoying.

#CodeSmell

back to gemlog

¹ Some people think that checked exceptions are a language smell, but language isn't a choice available in code maintenance.