Take the 2-minute tour ×

Excessive use of magic numbers or string literals in code is something of a code smell; not necessarily wrong but worth considering carefully. However, one can set up your editor/IDE to highlight string and number literals in garish colours like pink on lime green. That way, you can't help but notice where you are using these types and, hopefully, this practice will force you to consider carefully how you structure your code and perhaps avoiding the code smell, or concentrating it in well-defined places (like declaring all string literals as constants). Code deodorant if you like.

Another example practice is to use a large font size in your editor. This means that a smaller number of lines of code are visible on the screen which makes writing excessively long methods and classes more difficult - effectively guiding you towards writing short, purposeful functions.

What are other development/management practices which steer your programming in the right direction and help avoid programming code smells? See http://stackoverflow.com/questions/114342/what-are-code-smells-what-is-the-best-way-to-correct-them for an excellent catalogue of smells.

share|improve this question
I think the very first step is to not use the phrase "code smell." –  BobbyShaftoe Feb 19 '09 at 0:39
Yes, why is that phrase in every other topic these days? It's not a religion. Sometimes 'smelly' code does exactly what the customer wants and does it well. –  Ed S. Feb 19 '09 at 0:46
Don't use string literals at all?! Are you kidding? –  Ed S. Feb 19 '09 at 0:47
"code smell" is one of those slang expressions that is really quite silly, like referring to your development tools as a "stack" ;-) –  Steven A. Lowe Feb 19 '09 at 2:42
Code smell is a great way to say that the code is almost certainly going to be a maintenance problem when these structures are used heavily. If used just once, you don't get much of a smell, but if used a lot, the can destroy your maintainability. –  Bill K Feb 19 '09 at 22:32

9 Answers 9

Many descriptions I've seen for the process of using Code Smells is to first write code so it works, then refactor it based on the smells. In other words, don't spend a lot of time planning out your design anticipating smells and avoiding them (although obviously your habits and patterns will improve over time) but get something out there and then improve it.

Otherwise you run the risk of incurring the wrath of YAGNI.

For those few not familiar, Martin Fowler's seminal book, "Refactoring", starts out with an extensive enumeration of what he calls "Bad Smells", and then proceeds to explain refactoring in terms of deodorizing those smells.

share|improve this answer

One thing I use to identify C&P code at a glance is to look for patterns in the code. Heck, even in the SHAPE of the code.

If you see any shapes repeating themselves in your code, you've probably got something that needs to be factored out.

if(var1 != null)
if(var2 != null)

That's enough of a pattern that it should look wrong to you (this example doesn't mean anything, just the fact that it's a pattern that repeats)

I've seen a LOT of code like this:

Btn1.addActionListener(new ActionListener() {
    action(ActionEvent e) {
Btn2.addActionListener(new ActionListener() {
    action(ActionEvent e) {

Where everything is identical except the method being called.

This type of repetition is really encouraged by anynomous inner classes, they are so common that programmers often forget that it can be better to create a full-fledged listener class.

In fact, in this case the listener should probably be a parent with two subclasses that forward the middle part to either method1 or method2 depending on which class was instantiated and added as a listener.

share|improve this answer
A very advanced (though heuristical) way to remove duplication at a high level of abstraction. I don't understand why this answer hasn't been upvoted yet at all... –  thSoft Apr 20 '10 at 8:16

"Code smells" is concept pioneered by the Agile Community. I would suggest using rest of the agile approach along with this concept. Just about every programming pattern and every refactoring approach comes out of one or another code smells, see Martin Fowler's book, Refactoring. However, agile development using refactoring is about producing something that works now and then improving it over time rather producing pure code. So you will get problems in your code (Of course, every other method also also will get you problems too). The advantage of agile is that you refactor as you go, using your user stories as a guide for which way to go, so your code gets better despite never reaching perfection.

-- and you did write tests to allow you to refactor easily, did you not?.

share|improve this answer

A lot of the practices listed here also apply to removing code smells:


share|improve this answer

I am not sure marking all string literals in garish colors will help - thnk of messages in non-user-visible exceptions and asserts. They are one-time strings, very useful and you would not want to refactor them into variables.

An approach that consistently works is self-discipline and code reviews. You should get that uncomfortable feeling on seeing literals spread around, esp. those used more than once.

share|improve this answer

I run Source Monitor regularly on my code. Every time I see a comlexity value greater than 8.0 I start refactoring.

share|improve this answer

If you start seeing any of these, it's time to refactor:

  • Hard coded values which should be stored in configuration instead.
  • Try/Catch blocks used as logic
  • Methods which have grown too large.
  • Classes that do too little
  • Classes that do too much
  • Code that does not observe the DRY principle
  • Improper encapsulation such as inappropriately publicly scoped methods and/or properties
share|improve this answer

Wouldn't code deodorant be analogous to making bad code smell good...ie disguising the problem? Also, isn't your suggestion to highlight the hard-coded values more code-oderant rather than deodorant?

To me, avoiding code smells is a matter of writing good code over time by refactoring refactoring refactoring. I don't think anyone gets it right the first time...but code smells usually result from code that no one's refactored after the original purpose of that code no longer meets the current requirements.

share|improve this answer
Maybe what we need is code anti-persperent ;) –  Kirschstein Apr 21 '09 at 16:42

What i've been doing happens at commit time. Before I commit a change, I go through the diffs, comparing before and after. I comment on each change in the commit message as i go along.

If I can't figure out why something is different well enough to explain it in the commit message, then it's not ready to be commited. I'm not talking about other peoples changes, either. It is always me that made the change.

Either I refactor that piece of code until it makes sense, or it just doesn't go into the commit.

share|improve this answer
+1. This is equivalent to giving your own code a code review, and I've found it works really well for me. Since I'm only looking at diffs, I find it's easier to be objective about it. –  Daniel Pryden Sep 16 '09 at 17:31
Yes, I do this religiously too, and it works wonders. –  Zubair Feb 20 '10 at 9:10

Your Answer


