Code Smells

I'm often asked why the book Refactoring isn't included in my recommended developer reading list. Although I own the book, and I've read it twice, I felt it was too prescriptive-- if you see (x), then you must do (y). Any programmer worth his or her salt should already be refactoring aggressively. It's so essential to the craft that if you have to read a book to understand how it works, you probably shouldn't be a programmer in the first place.


This is a companion discussion topic for the original blog entry at: http://www.codinghorror.com/blog/2006/05/code-smells.html

There’s also a nice, simplified taxonomy of code smells here:

http://mikamantyla.eu/BadCodeSmellsTaxonomy.html

He (correctly, IMO) groups the smells into five general categories:

  1. Bloaters
  2. OO Abusers
  3. Change Preventers
  4. Dispensables
  5. Couplers

I found “How To Write Unmaintainable Code” about a year ago, and I could not stop laughing. Re-read it this morning, and I still can’t stop :slight_smile:

Thanks, Jeff!

"Overload and Bewilder
In C++, overload library functions by using #define. That way it looks like you are using a familiar library function where in actuality you are using something totally different."
Or, overload it to have an A and a W version… :slight_smile:

“The more parameters a method has, the more complex it is. Limit the number of parameters you need in a given method, or use an object to combine the parameters”

Im just curious – doesnt this violate some principle by having objects to tightly coupled?

Hey, I know that Cheese Shop! It’s the one in Rockridge right next to Bart! I bet you sip your latte and refactor away in the caffe next door, with a splendid chunk of Osau Iraty and Triscuits you lucky dog…

Sorry, nothing much else to contribute :).

Argh, I accidentally deleted Jeff Perrin’s comment while cleaning up blog spam. My apologies, Jeff.

As I recall, Jeff said that I erred in calling Refactoring prescriptive. He said that the book actually encourages you to be flexible.

I rarely pull rank on my team, but this time I’ve made this post required reading. Great stuff.

That said, there’s only one smell I’m not quite sold on. Data Classes. I’ve found them quite handy when passing between classes (i.e. Long Parameter List). Also useful when you have a parent / child type relationship, and are basically just storing the data in a collection for reference or display.

Also I thought Ron Ruble made some interesting points about maintenence coders. Often many get their start that way, and are told to get in and get out as quickly as possibe with minimal impact to the code. More changes require more testing, and all management wants is to get it fixed as quick as possible, not sponsor a rewrite because it’s “smelly”.

Thanks again, great post!

Robert

Actually, Jeff, the “code smells” metaphor came out of a discussion between Martin Fowler and Kent Beck while Fowler was still writing the Refactoring book.

Together, they wrote chapter 3, “Bad Smells in Code,” which is not prescriptive at all, but does present a catalog of smells.

You may not be impressed by the refactoring catalog itself, but IMHO you should at least add the first four chapters to your recommended reading list.

1 Like

I found the book most valuable for the non-reference part; for instance, the discussion about how to explain just what you’re doing when you’re refactoring was helpful for me.

I also liked the example he used at the beginning, which seemed more realistic and creative than a lot of the book examples I see (e.g. Kent Beck’s Money example in TDD by Example). I did my first test-backed refactoring by working through Fowler’s movie rental code.

To me Refactoring is not “You see this then do this” but more “If you want to this change, here are the steps to do it safely and reliably”.

Don’t get me wrong, your list of code smells are good but they what I would call anti-patterns. I.e. patterns of bad code that should send up a red flag.

I am currently having to deal with, Long Method, Conditional Complexity, Large Class, Refused Bequest, Inappropriate Intimacy, Shotgun Surgery and Oddball Solution. Its pure hell.

Great article, but I also have a hold out for targeted usage of DataClasses; they have their uses when all you need to do is buffer a “record” style collection; particularly if a “record” contains one or more internal collections as well.

Surely the cheese shop in the photot is the Neals Yard Dairy? Purevyors of the famous Stinking Bishop…

Some points of disagreement:

  1. Assuming that you are right in that most of us know how to refactor, I think that’s because experience gave us the needed knowledge. Time and pain was needed. This time and pain, as in any knowledge area, can be reduced by reading books, like Fowler’s.

  2. Apart from this, I think even a developer with experience and knowledge of most refactorings could miss some smells. We are humans and we acquire bad habits.

  3. A big part of the knowledge gained with experience is too intuitive and that brings some problems. When things are intuitive and you suddenly find someone that doesn’t share your intuition you have to fight back with solid explicit arguments. Fowler’s book helps you ease the work needed to find those arguments letting you show clearly why your intuition is the way it is. You can do it yourself of course, but it’s easier to reuse the effort someone else did, if you share it of course.

  4. The book defines a vocabulary to deal with our intuition or implicit concepts, that is very very important. Now we can share it and we can communicate more efficiently. The same advantage we discovered after design patterns appeared.

  5. You agree with Fowler that smells are important. The book is a reference to smells. IMO refactoring is more about identifying design problems (smells) than about the relatively simple things needed to make them disappear, but it’s both of course.

  6. Things that are important, like smells, must be made explicit so the problem can be easily studied and possibly build some more knowledge on higher level concepts. One of those higher level concepts that could be further developed was the development philosophy of Continuous Design in which a core concept is refactoring.

  7. I think there’s a confusion between simplicity and importance. Not only complicated books are important.

So, what’s the difference between the “middleman” code smell and the “facade” design pattern?

The link to How to Write Unmaintainable Code was blocked by our firewall with a nice big red ACCESS DENIED. Category: Computer Crime.

Had me rolling in my cube for a while.

I think you’re just jealous you didn’t write it? :slight_smile:

The Fowler refactoring book is great for junior developers - in saying it’s “too prescriptive” I think you completely miss the point -
what’s important is that it introduces the object thinking that underlies his process.

It’s a great introduction to object thinking and more complex issues such as use of interface inheritance and test driven development - would recommend it to my junior devs… it should be required reading for all .NET developers

The first time I read Fowler’s book my response was “eh” – most of the refactorings described stuff I’d been doing for years, stuff that should be second nature to any competent programmer. It was only when I first started using an IDE with good refactoring support that I realized the true value of the book: Naming the refactorings. Not unlike patterns, in that respect.

Ward Cunningham also has a nice set of code smells at his site: http://c2.com/xp/CodeSmell.html

If a code stench is a really bad code smell, what would we call an often recurring smell… a code stink?

/J