Skip to main content

Your code smells? Let it!

Code smell is a term used pretty commonly in software development.  It is a way for one developer to express to another that they're experience is telling them something is wrong, even though they don't see it on the surface.  In fact, that's what it's supposed to mean.

But software developers don't do well with "trust your gut" as a methodology, so we make an effort to identify and quantify things like this.  For that reason, over time we have managed to put together lists of things in your code that are "code smells" and explain why we don't like them.

For illustration, I'm going to refer to personal experience and talk about an application I'm currently working on.  A common code-smell that developers will talk about is singletons.  So that will be the basis for my example, but the message should be clear even without the example.

Why does it smell?


The first thing you should ask the developer on your team who's most knowledgeable about the code in question (that may be yourself) when you encounter a code smell is "Why does it smell?".  This is the part I see skipped--A LOT.  This is the most important part of the process.  Let's face it.  There are legitimate reasons for things to smell.  Your mechanic's shop should not smell the same as a bank vault!  Each code base is going to have its own set of smells, and that's ok, as long as the answer to "Why does it smell?" is reasonable.

In a project I've stepped into, singletons are everywhere.  I asked one of the developers about them and his answer was one sentence.  "We have memory constraints."  Oh!  That makes sense.  This application lives on (crappy) webservice connections, and is written in an older language, so it's logical to store those connection objects for reuse later, rather than waiting on a crappy garbage collector to delete them and running out of memory.  This code base smells like they're using singletons as global objects.. Guess what?  They are!  And it's OK.  That was their intent!

Of course, the answer is not always valid.  If the answer is "That's the way it's done" or something else vague like that, it's time to investigate yourself.  It may be that there is a legitimate reason and you haven't encountered it yet, but this is a code smell, and so you need to find the source.  You cannot, and should not go changing code to "fix the smell" if you don't know why it smells.

In my example, if I had started getting rid of singletons to "fix it" and making them instances of the service all throughout the application, I would have surely crashed production!  Don't go fixing things that you don't KNOW are broken.  Code smells are just that: a smell.  Some smells are supposed to happen!

What if it's bad?


So you've identified a code smell.  You asked "Why does it smell?" and you figured out, through some form of investigation, that it's bad.  This is the smell of burning rubber over a box of wires.  It's a bad smell, we need to fix it.

You're not supposed to fix the smell!

So many developers get this bit wrong.  The smell is your marker.  It identifies that there's a problem in your code base.  The smell itself is NOT the problem!  99% of code smells are pointers to an architectural problem!  The smell "goes away" when you fix the root problem.  Saying "x is a code smell" is not the same as saying "x is bad".  The code you're looking at solved a problem.  It is good code.  It just may not need to exist if you fix the part of the code that is bad.  The part that caused the smell to begin with.

As an example, in my current day job, they identified the singletons but knew that "singletons are a code smell".  They made the standard mistake.  They think that the word code smell means "bad".  So they say "singletons are bad".   Then they try to remove the singleton.  This actually happened in this code base.  They did something I'm seeing a lot on blogs.  They wrapped their singletons in a Dependency Injection library and pretended that, since the instance is being passed to their class via dependency injection, it's no longer a singleton.

Don't hide the smell!


Ok, so the new guy is just starting.  He didn't do any of the above, but he saw the singleton pattern and got all excited to help and swapped it out for dependency injection...  What did he do?  He masked the smell.  The smell is there for a reason.  It's like covering up the dog turd in the middle of the floor by putting a towel over it.  Whether this is a kennel, where that dog turd needs to be kept in the dog-turd specific area (maybe a singleton resolver class or some such), or a perfume store, where it needs to be cleared out immediately at all costs, he has made it harder to fix, and hard to identify for everyone.

In a code-base where the smell identifies an actual problem, to fix the smell masks the real problem and keeps you from finding it longer.

In a code-base where the smell points to outside forces beyond our control, the fix makes it harder to maintain, and harder for new developers to step in and learn about the code base.

Never, ever hide the smell.

What about your singleton problem?


With a big problem in an existing code-base like this, I'm starting small.  First thing to do, unmask all the singletons.  I'm creating a group of static classes that wrap up the singletons in the application.  Some day in the future, I'll be able to remove the Dependency Injection framework entirely and start over, using it correctly (for actual dependency injection).  But until then, I've had to think differently.  For now, in this particular code-base, whenever I see that dependency injection is being used.  That, for me, is a code smell.

Comments

Popular posts from this blog

Managing Developers is HARD

I've been a software dev for a long time.  I've also been running my own software company for a few years now.  This is important information because of why I do these things.  I am a sofware developer because I love learning.  I slack off when doing a job that bores me, and software development always has something new to experience which keeps me excited and interested.  Why start a software company then?  That puts me in the role of manager rather than developer.  The truth is simple.  I've worked for a lot of companies, and I don't see any of them doing a great job of managing their software development.  That's not to say none of them have done a good job, but no one out there seems to be doing a great job.

How are they different?
A lot of companies get this part right.  Software developers are different from other employees.  The distinction is important in the same way it's important to acknowledge that an insurance agent is different from a construction…

When Is Software Done?

I have some very exciting news.  A piece of software I've been working on for over 2 years is released to the general public!  This is a little exciting if it were software I'd been working on for some big company.  It's very exciting because it's software I have been working on for my company.  That's right!  My company is ready to start selling software and start making money!

I'm not gonna use this blog post to talk about my company and what it does.  You can read about that in our press release.  Instead, I'm going to talk about the software industry and the concept of done.  Because, as with everything, it's more complicated than it seems.

Software is never really done
Actually that's a misnomer.  Software can really be done.  But done is sort of a quantum state--there and not there at the same time.  First and foremost, anyone can understand that software that works is complete.  If the software's purpose is to process a credit card, if th…

How to identify a skilled programmer during an interview

How does one identify a skilled programmer?  No company that has interviewed me could tell the difference between myself and other programmers they'd interview.  The interview process is truly a game of luck in this industry--on both sides.  Both the programmer and the company are basing their actions entirely on luck.

Companies have come up with numerous methods to attempt to discern a good programmer from a bad one.  The best tricks they have include a series of math problems, algorithms, problem solving technique tests, and even obscure programming questions, some without definitive answers.  As an example: Is there an authoritative source of information on the core principles that define object oriented programming?  I've heard everywhere from 3 to 7.  In a field of research about a synthetic concept, an authoritative answer is almost impossible to obtain.

Programmers were then forced to study to the interview.  Careercup is one of my favorite sites for this.  This almost …