melreams.com

Nerrrrd

Code smell: temporary fields

in code smell, development, software design

Several puffins are clustered on a grassy hill or shore while two more puffins come in for a landing.
Unrelated image from pexels.com to make this post look nicer in social media shares and also because puffins are cute.

Today’s code smell is temporary fields. Like with all code smells, sometimes these aren’t a problem at all. For example, it’s pretty normal to have a user object with a bunch of optional fields. Sometimes you have users who just start out with a username and a password and fill in their profiles later and sometimes you have fields for stuff like social media accounts that only some users will ever fill in, but sometimes fields that only sometimes have a value are a problem.

One of the ways people try to fix the problem of a long parameter list is to make some of those parameters into fields and then set them later if they need to. Unfortunately that’s not really any better because it just hides some of the confusion under the rug instead of actually getting rid of it. Instead of it at least being obvious you have too many parameters, now you have a bunch of fields that may or may not get used depending on what the object is up to and you have to go hunting to figure out what’s going on.

Since professional programmers read code much more often than we write it, it’s a huge waste of time to have to go on an exploratory mission every time you need to understand that code again. If you do end up needing to update that code, best of luck figuring out which fields are important and whether your refactor / bug fix / new feature broke anything.

I had a heck of a time finding a code example for this code smell but fortunately so did Mark Seemann, who was frustrated enough by that to write not just an example of the smell but of how to refactor it too.

If you find this smell in your code, what do you do about it?

Pulling out the temporary bits into their own object and just using that is a good option (if that works for your code, of course). Isolating the fiddly bit makes the rest of the object cleaner and makes it clearer what the fiddly bit is up to. You can also add a null object (a special object where you put whatever defaults you use when one of your temporary fields is null). That way your normal processing code can be clean and simple instead of littered with null checks. Sometimes that just makes things more complicated, so it’s a bit of a judgement call.

If you’re having trouble figuring out what an object even does because most of its fields are null most of the time, see if some refactoring helps. You can always put it back the way it was if it’s worse afterwards :)

Code smell: feature envy

in code smell, development, software design, Uncategorized

A large rock or very small island with a few green shrubs growing on it surrounded by greeny blue water and white surf.
Unrelated image from pexels.com to make this post look nicer in social media shares.

Today’s code smell is feature envy. Feature envy is when one class uses the methods of another class to excess. It’s called feature envy because classes that use methods from another class excessively look envious of that class’s features.

The problem with that, as usual, is the way your logic gets spread around between multiple classes. If you have one method that’s using another class so much that it’s hardly even separate from it, you’ll probably have to change things in both places when you need to fix a bug or add a new feature, but the compiler can’t tell you that you need to do that so you risk creating a shiny new bug by missing a spot.

Feature envy probably makes more sense with an example. Let’s say you have an address class and a customer class, and when you call customer.getMailingLabel() your customer class calls address.getStreetNumber(), address.getStreetName(), address.getCity() and so on to build a complete address string for printing a mailing label. Let’s imagine that policy changed and you’re now able to ship to rural mailboxes, so you need to add route and box numbers to your address and to use those instead of street number and street name if they’re present. If your code has feature envy, you have to change both the address object and the getMailingLabel method to support rural mailboxes. If you moved the getMailingLabel method into the address class, then you would only have to change one class when you wanted to change how you build mailing labels.

As with just about all code smells, feature envy isn’t always a sign there’s a problem. Sometimes you want to separate your data from operations on it, especially if you’re using a strategy or visitor design pattern. If, for example, you want addresses formatted differently for different reports, the formatting is probably more the job of the reporting code than the address object. In that case it would make more sense for a visitor to contain the formatting code so the address object doesn’t have to care about all the different reports.

Code smell: switch statements

in code smell, development, software design, Uncategorized

a frying pan full of brightly coloured vegetables cut into small pieces
Unrelated image from pexels.com to make this post look nicer in social media shares.

Today’s code smell is switch statements. Long switch statements or complicated if statements often (but not always!) mean that something has gone wrong with your design. Objected oriented languages give you a lot of tools to avoid big complicated switches, if you aren’t using one of those there needs to be a good reason for it. If you’re not using an object oriented language, on the other hand, I’m honestly not sure if switch statements are a sign of a potential issue. If any of my readers could weigh in on that, that would be really helpful.

But why are switch statements a problem? Because you’re spreading around logic that should probably be in one place, and if you add a value to one switch statement you’re likely to need to hunt down all the other ones and change them too.

If, for example, you have a switch statement that does different calculations based on the type of the object passed in, that logic for how to do the calculation probably belongs inside the object passed in to the switch. If you change that object, you’ll have to hunt down every place it’s used and make sure nothing else needs to be changed, which is obviously asking for trouble :) If you give each object a doCalculation method and replace the switch statement with a simple call to doCalculation, then all of the logic for that object stays inside the object. With all your logic in one place, you don’t have to search for every thing you need to change and risk missing a spot, and your design just makes more sense when logic for one thing isn’t smeared across multiple objects.

Polymorphism isn’t the only way to fix a switchy smell, if your method switches on a parameter that’s passed in, you may want to change it to separate named methods to more directly do the thing. If you have a setProperty(String name, String value) method, you could replace it with named methods like setType, setModel, setYear, etc. You might end up with a lot of little methods, but it’s still clearer than one setProperty method with a giant switch. If you end up with a truly excessive number of methods, that’s another code smell that points to your object doing too much.

Just because switch statements are sometimes a problem doesn’t mean they always are. Sometimes what you’re doing is so simple that an interface and a set of objects that implement it (or changes to your existing interface and objects) makes your code more complicated rather than less. And sometimes there just isn’t a better way to do it: factory methods and abstract factories rely on switches or a series of ifs to figure out which implementation to return.

What’s a code smell, anyway?

in best practices, code smell, development

a rugged metal tower, possibly a lighthouse, seen at low tide on a rocky beach with puddles all around
Unrelated image from pexels.com to make this post look nicer in social media shares.

Design is one of my favourite parts of programming, but I constantly run into the problem of how to tell whether my design is any good.

Before we go any further, I think we should define what makes a good design is. My definition is that if you can change it in the future without cursing your past self, it must have been good design. You may have noticed that using that definition makes it difficult to figure out whether a design is any good until potentially months after the fact when you need to change it. It would be nice to know ahead of time whether your design is going to lead to a lot of swearing in the future, and that’s where I have trouble.

To be fair, if I could definitively answer the question of whether a design was good without having to try to change it, I’d be a millionaire and I’d be writing this post from my own private island :) Whether a design that seems good now is going to continue to be helpful in the future is just a hard problem and there’s no getting around that.

That said, I think code smells can help a lot there. A code smell is, in the words of Martin Fowler, a surface indication that usually corresponds to a deeper problem in the system. The term was coined by Kent Beck while he helped Martin Fowler with his book Refactoring.

This whole concept would probably make more sense with an example. An especially designy code smell is a class that does too much. Ideally a class should have one responsibility – if it’s concerned with more than one thing that’s a strong sign it should be broken up into separate classes. The specific problem with a class that does too much is that it’s likely to lead to long, messy methods that do too many things and don’t make sense because of it, and it generally makes it hard to reason about your system. The more one class does, the more state it’s likely to need to keep track of, and the more state one class keeps track of, the more chances you have to completely mess it up. You may have gotten a hint here that software design is about accepting that humans are bad at it and trying to work around that basic fact :p

The thing with a code smell is that it’s not a hard and fast rule, it’s just a hint that something could be wrong. Sometimes it can be reasonable for a class to do “too much,” it depends on the exact thing you’re trying to accomplish and the context (i.e. the rest of your application) that your class is part of. Having a good design is more about the thing you’re trying to accomplish than it is about following all the rules whether or not they make sense in your particular situation.

The great thing about code smells and design patterns and all that is that they let you take advantage of other people’s experience. Can you imagine how long it would take to get good at software design if you had to make all the mistakes yourself? I love hearing what other people have messed up so I can maybe avoid that particular problem. And from the other side, I’d love it if anything I’ve told another dev helped them avoid a problem. It’s all well and good if I solve a problem for myself, but if I can help a couple of people avoid the problem in the first place and if those people help a couple other people avoid that problem, then I’ve done much more good than if I quietly fixed my own code and moved on.

Stay tuned for more posts about code smells, there are plenty of them to get through :)