Code Smells

You said:

I 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. /I

Looking at the popularity of the book in programmers community, your statement is suggesting that most of the programmers are not programmers.

You may find it humourous that when I saw the smell for ‘Dead Code’, I instictively tried to vote it up. Spending too much time on StackOverflow.

I like the Refactoring book primarily because it gives names to the stuff I already do. The main advantage of that is communication with other people:

  • for junior programmers, the book gives them the steps on how to do things
  • for senior programmers, the book allows me to talk with other senior programmers that read the book with less words.

Comments: I like to know the what in the method javadocs normally, because I don’t want to go through the code to figure what it does especially if I am just a consumer of the interface. I do understand the importance of the why as well.

Conditional Compexity: sometimes is necessary. (Comments come into play) writing another API to do validations for low level things (e.g. match string if provided times 10 search keys) may yield to something that looks complex, but I see no need to refactor this as it will make things harder to debug if you’re going across several classes when having everything in one validate method is sufficient.

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

http://jaber.mysinablog.com/
http://blog.qooza.hk/jaber

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…

The link to How to Write Unmaintainable Code (http://thc.org/root/phun/unmaintain.html) doesn’t seem to work any more - I presume this is same article: http://freeworld.thc.org/root/phun/unmaintain.html

Got confused between “middleman” code smell and facade design pattern
"middleman" code smell correction contradicts with facade design pattern implementation.

Any programmer worth his or her salt should already be refactoring aggressively.

But as a practical matter, many organizations actively inhibit refactoring, and teach the programmers that “this code is too complex to change.”

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.

A little overzealous, in my opinion. -If- the programmers have been trained correctly, they should have picked up some form of refactoring skills. But in many cases, they haven’t; they’ve even been trained that old, crufty code is gold and must -never- be changed.

I don’t feel it’s reasonable to imply all these coders are a dead loss because they got started on the wrong foot.

There’s nothing wrong with codifying refactoring guidelines in a book. But the most important
guideline is to watch for warning signs in your own code-- so called “code smells”.

If your job is 50% writing your own code. Lots of maintenance programmers write very little code from scratch, and spent almost all there time working with someone else’s crufty code.

Developing your “code nose” is something that happens early in your programming career,
if it’s going to happen at all.

9 times out of ten, you’re right. But every time someone says something like this, in any field, someone appears as an outlier on the skills graph.

You make some good points; I just feel you’ve overstated your arguments on this one.

I think that refactoring alone solves only part of the problem. To really make the code better you must have a good understanding of unit testing and patterns in addition to refactoring. I don’t feel comfortable refactoring code that has no unit tests so the first thing I do when I have to work on someone else’s crud code is to add unit tests.

I know this post is very old, but there is a smell I didn’t see documented here which is writing code to trick the compiler. Like an else statement with a condition that will never fire but pleases the compiler.

This is good information.

However, whenever it comes to smells, I struggle to remember their cryptic names. Moreover, there was no single comprehensive catalog available to refer them at one place in detail with examples (and hopefully with some cues how to refactor them).

A nice treatment of design smells could be found in a book viz. “Refactoring for Software Design Smells:Managing Technical Debt”. The classification (based on fundamental principle) looks interesting and intuitive to me. Most importantly, it describes the smells in great detail (that is what I was looking for).

Regarding advices that talk about Object construct patterns, there are so many words about Classes.
Someone would ask - but what if I use prototype inheritance based language or something else that doesn’t provide Classes as object construct definitions?
Thankfully, it’s clear enough that you can apply Jeff’s advices to anything, and especially to languages such as Javascript, Rust and Clojure, but I think we’re all somehow far away from describing complex logics behind our programs’ architecture at certain abstract views that could be taken as general advices. Even the sentence behind this is complex enough to be parsed in one take.
Simply put, my thinkings are, that we should be even more abstract at giving advices like these, but with more intuitive views, that even kids would be able to grasp.
The only real problem is that the more you get abstract, the more those that should follow those abstracts tend to neglect them as they can’t stick with them all the time during the real programming, and by the time they write a few lines of code they’re one leg in quagmire. Programmers need real and concise examples like Jeff’s that would ring cow bells in their heads while they’re doing a small change in their program but are actually changing it in 30 different files.
Sometimes, on projects that take more than a single brain power, upfront architecture is enough to decouple components and provide elegant design for the code, but it usually doesn’t last for long and programmers that weren’t considering architecture and refactoring tend to create mess.
How then do we make them understand these topics on a more general way so they can have better understandings right from a start and thus don’t need real code examples?
Do we design a programming language or tools that won’t forgive things such as tight coupling? Or do we make better approaches to learning programming and software architecture? Or something else?

I believe this is an extensive list every programmer must be aware of.

Moreover, due to lack of proper exposure learning design patterns might get complex and being aware of each might not be possible for newbie programmers.

However the points mentioned here are so to the point and are applicable for the smallest of projects one would ever build. This practices can slowly lead a developer to learn design patterns. One would start finding the common design approach and related it just by going through this points and comparing their code.

Thanks for sharing this.

Hi Jeff, anyway you can fix the formatting on this page? The first half of the code smells have squished titles (in the grey box).

Thanks!

1 Like

Thanks Meyer I just rebuilt it – I appreciate the reminder!

Hi,
I"m looking for a tool that could detect Refused Bequest.
You know any?

Thank You

Where is the archive of the blog these days? This link doesn’t work.

Hey,

The How to Write Unmaintainable Code link no longer seems to work, so I did some google-fu. The original is published here http://mindprod.com/jgloss/unmain.html but the one on github https://github.com/Droogans/unmaintainable-code might be more easily read/navigated/searched.

Cheers,
Attila

1 Like

Speculative Generality is the worst smell.

When the first day’s coding is creating 24 folders which match Java Enterprise file structure. You know there’s going to be trouble.

1 Like

The most important thing to remember about code smells is that they’re heuristics, not hard and fast rules. People have already pointed out that the Facade pattern looks a lot like a Middleman smell. So does the object adapter version of the Adapter pattern, or a convenience base class for decorator implementations. Parameter objects are most commonly data objects (though sometimes you may want to put functionality on them too)–this applies both to objects created to prevent long parameter lists/primitive obsesson/data clumps and those that only handle one primitive and just exist to help type safety (e.g., a wrapper class with a name explaining the intended function of its primitive value).

Basically, when code matches one of these definitions it’s a yellow flag–is the fact that it’s violating a code smell rule serving a legitimate purpose, or could the smell be fixed without other unpleasant consequences? If the latter, by all means fix it, but don’t expect to be able to go down a checklist like this and mechanically reject code.

2 Likes