A “Steaming Pile of Code Smell” Kata?

I recently came across Sandro Mancuso’s “Trip Service Kata”  and the accompanying video. It’s inspiring to see how he systematically and safely performs surgery under test to perfect some heretofore untestable code, but I can’t help feeling that the code in the kata is not ugly enough. I’ve seen way worse. I’ve probably written way worse before Craftsmanship love came to town. Sure, there are a couple of static calls, some feature envy, etc., but I can think of a lot of ways to make this code much harder to test and refactor.

That got me thinking that maybe I should create a sort of “Bar Mitzvah Cocktail” kata (or “Steaming Pile of Code Smell” kata), where the idea is to take this Trip Service code and make it as ugly as possible. A sort of Bullwer-Lytton contest for code, or a sort of “Brewster’s Millions” approach to become disgusted with one’s baser coding instincts by doing the opposite of clean coding until it becomes unbearable. But I think there’s enough bad code in the world already, and what I just described might in fact be your day job. 😉

It might, however, be a fun mental exercise or even a good job interview question to ask, in what ways could I make this code more difficult to test? Here are a few of my ideas. Feel free to add yours in the comments.

• Longer methods. The methods in the kata are too short. To be really awful, a method needs to span at least 100 lines.
• Reusing variables for different tasks. With long methods comes the chance to create confusion and make automatic method extractions impossible. If you use a variable in one loop, why not reuse it in another loop that does something entirely different? Or at least you can store the results of one loop in several different variables and reuse those variables in later loops for somewhat related tasks.
• Static calls in the constructor… of a superclass. When some Craftsmansip coach tries to instantiate the class in a unit test, kaboom! Bonus points if the static call in question alters the database or debits a credit card.
• Deep inheritance hierarchy. Don’t make it too easy to find the superclass constructor which contains the static call.
• Mixing presentation logic with business logic. How about a javascript alert if the logged-in user is not allowed access to the trips? Maybe change the color of a button while you’re at it.
• Mixing persistence code (SQL requests, for example) with business logic. Bonus points if the business logic generates fragments of a SQL request and passes them to another method.
• Huge switch statements. Preferably omitting some “break” statements intentionally.
• Calls via reflection. Maybe the switch statements could sometimes change which method will be called via reflection. Bonus points if the name of the method called by reflection is unintelligible.
• Methods which have multiple side effects… via static calls, of course. Bonus if the method normally returns null.
• Bad names. How about “TrpMgr” instead of TripService? Too understandable? And how about a single letter name for any local variable or method argument?
• Law of Demeter violations. A long method should have to fish around to unearth the objects it needs.
• Other abstraction leaks. Maybe mix into the business logic some cross-cutting concerns, like logging and generating statistics.
• Use labels generously in for loops.

That’s all the bad ideas that come to mind that would not require a bug fix (such as security holes or exponentially slow algorithms). The idea is that you can add things to the spec (after all, spec changes are the main trigger of code smells), but the code should do effectively the same thing when it’s cleaned-up as it did at its dirtiest.