Ruby/Rails Code Smell Basics 03

This newbie-friendly article covers another round of smells and refactorings you should familiarize yourself with early in your career. We cover case statements, polymorphism, null objects and data classes.

Topics

• Case Statements


• Polymorphism


• Null Objects


• Data Class

Case Statements

This one could also be named “checklist smell” or something. Case statements are a smell because they cause duplication—they are often inelegant as well. They can also lead to unnecessarily big classes because all these methods that respond to the various (and potentially growing) case scenarios often end up in the same class—which then has all kinds of mixed responsibilities. It’s not a rare case that you have a lot of private methods that would be better off in classes of their own.

A big problem with case statements occurs if you want to expand them. Then you have to change that particular method—possibly again and again. And not only there, because often they have twins repeated all over the place that now need an update as well. A great way to breed bugs for sure. As you might remember, we want to be open for extension but closed for modification. Here, modification is inevitable and just a matter of time.

You make it harder for yourself to extract and reuse code, plus it is a ticking clutter bomb. Often view code depends on such case statements—which then duplicate the smell and open the gate wide open for a round of shotgun surgery in the future. Ouch! Also, interrogating an object before you find the right method to execute is a nasty violation of the “Tell-Don’t-Ask” principle.

Polymorphism

There is a good technique to handle the need for case statements. Fancy word incoming! Polymorphism. This allows you to create the same interface for different objects and use whatever object is needed for different scenarios. You can just swap in the appropriate object and it adapts to your needs because it has the same methods on it. Their behaviour underneath these methods is different, but as long as the objects respond to the same interface, Ruby does not care. For example, VegetarianDish.new.order and VeganDish.new.order behave differently, but both respond to #order the same way. You just want to order and don’t answer tons of questions like if you eat eggs or not.

Polymorphism is implemented by extracting a class for the case statement branch and moving that logic into a new method on that class. You continue to do that for every leg in the conditional tree and give them all the same method name. That way you encapsulate that behaviour to an object that is best suited to make that kind of decision and has no reason to further change. See, that way you can avoid all these nagging questions on an object—you just tell it what it’s supposed to do. When the need arises for more conditional cases, you just create another class that takes care of that single responsibility under the same method name.

Case Statement Logic

class Operation
  def price
    case @mission_tpe
      when :counter_intelligence
        @standard_fee + @counter_intelligence_fee
      when :terrorism
        @standard_fee + @terrorism_fee
      when :revenge
        @standard_fee + @revenge_fee
      when :extortion
        @standard_fee + @extortion_fee
    end
  end
end

counter_intel_op = Operation.new(mission_type: :counter_intelligence)
counter_intel_op.price

terror_op = Operation.new(mission_type: :terrorism)
terror_op.price

revenge_op = Operation.new(mission_type: :revenge)
revenge_op.price

extortion_op = Operation.new(mission_type: :extortion)
extortion_op.price

In our example, we have an Operation class that needs to ask around about its mission_type before it can tell you its price. It’s easy to see that this price method just waits to change when a new kind of operation gets added. When you want to display that in your view as well, you will need to apply that change there also. (FYI, for views you can use polymorphic partials in Rails to avoid having these case statements blasted all over your view.)

Polymorphic Classes

class CounterIntelligenceOperation
  def price 
    @standard_fee + @counter_intelligence_fee
  end
end

class TerrorismOperation
  def price 
    @standard_fee + @terrorism_fee
  end
end

class RevengeOperation
  def price
    @standard_fee + @revenge_fee
  end
end

class ExtortionOperation
  def price
    @standard_fee + @extortion_fee
  end
end

counter_intel_op = CounterIntelligenceOperation.new
counter_intel_op.price

terror_op = CounterIntelligenceOperation.new
terror_op.price

revenge_op = CounterIntelligenceOperation.new
revenge_op.price

extortion_op = CounterIntelligenceOperation.new
extortion_op.price

So instead of going down that rabbit hole, we created a bunch of operation classes that have their own knowledge of how much their fees add up to the final price. We can just tell them to give us their price. You don’t always have to get rid of the original (Operation) class—only when you find that you have extracted all the knowledge it had.

I think the logic behind case statements is unavoidable. Scenarios where you have to go through some sort of checklist before you find the object or behavior that gets to do the job are just too common. The question is simply how are they are handled best. I’m in favor of not repeating them and using the tools object-oriented programming offers me for designing discrete classes that can be swapped out easily through their interface.

Null Objects

Checking for nil all over the place is a special kind of case statement smell. Asking an object about nil is often a kind of hidden case statement. Handling nil conditionally can take the form of object.nil?, object.present?, object.try, and then some sort of action in the case nil shows up at your party.

Another more sneaky question is to ask an object about its truthiness—ergo if it exists or if it’s nil—and then take some action. Looks harmless, but it’s just a disguise. Don’t be fooled: ternary operators or || operators also fall into this category, of course. Put differently, conditionals not only show up clearly identifiable as unless, if-else or case statements. They have subtler ways to crash your party. Don’t ask objects for their nil-ness, but tell them if the object for your happy path is absent that a null object is now in charge to respond to your messages.

Null objects are ordinary classes. There is nothing special about them—just a “fancy name”. You extract some conditional logic related to nil and then you deal with it polymorphically. You contain that behaviour, control the flow of your app via these classes, and also have objects that are open for other extensions that suit them. Think about how a Trial (NullSubscription) class could grow over time. Not only is it more DRY and yadda-yadda-yadda, it’s also way more descriptive and stable.

A big benefit of using null objects is that things can’t blow up as easily. These objects respond to the same messages as the objects emulated—you don’t always need to copy the whole API over to null objects of course—which gives your app little reason to go crazy. However, note that null objects encapsulate conditional logic without completely removing it. You just find a better home for it.

Since having a lot of nil-related action in your app is pretty contagious and unhealthy for your app, I like to think of null objects as the “Nil Containment Pattern” (Please don’t sue me!). Why contagious? Because if you pass nil around, some place else in your hierarchy, another method is sooner or later also forced to ask if nil is in town—which then leads to another round of taking countermeasures to deal with such a case.

Put differently, nil is not cool to hang out with because asking for its presence becomes contagious. Asking objects for nil is most likely always a symptom of poor design—no offense and don’t feel bad!—we’ve all been there. I don’t want to go “willy-nilly” about how unfriendly nil can be, but a few things need to be mentioned:

• Nil is a party pooper (sorry nil, had to be said).


• Nil is not helpful because it lacks meaning.


• Nil does not respond to anything and violates the idea of “Duck Typing”.


• Nil error messages are often a pain to deal with.


• Nil is gonna bite you—sooner or later.

Overall, the scenario that an object is missing something comes up very frequently. An often cited example is an app that has a registered User and a NilUser. But since users that don’t exist is a silly concept if that person is clearly browsing your app, it’s probably cooler to have a Guest that hasn’t signed up yet. A missing subscription could be a Trial, a zero charge could be a Freebie, and so on.

Naming your null objects is sometimes obvious and easy, sometimes super hard. But try to stay away from naming all null objects with a leading “Null” or “No”. You can do better! Providing a little bit of context goes a long way, I think. Choose a name that is more specific and meaningful, something that mirrors the actual use case. That way you communicate more clearly to other team members and to your future self of course.

There are a couple of ways to implement this technique. I’ll show you one that I think is straightforward and beginner-friendly. I hope it’s a good basis for understanding more involved approaches. When you repeatedly encounter some sort of conditional that deals with nil, you know it’s time to simply create a new class and move that behaviour over. After that, you let the original class know that this new player deals now with nothingness.

In the example below, you can see that the Spectre class asks a bit too much about nil and clutters up the code unnecessarily. It wants to make sure that we have an evil_operation before it decides to charge. Can you see the violation of “Tell-Don’t-Ask”?

Another problematic part is why Spectre would need to care about the implementation of a zero price. The try method also asks sneakily if the evil_operation has a price to handle nothingness via the or (||) statement. evil_operation.present? does make the same mistake. We can simplify this:

class Spectre
  include ActiveModel::Model
  attr_accessor :credit_card, :evil_operation

  def charge
    unless evil_operation.nil?
      evil_operation.charge(credit_card)
    end
  end

  def has_discount?
    evil_operation.present? && evil_operation.has_discount?
  end

  def price
    evil_operation.try(:price) || 0
  end
end

class EvilOperation
  include ActiveModel::Model
  attr_accessor :discount, :price

  def has_discount?
    discount  
  end

  def charge(credit_card)
    credit_card.charge(price)
  end
end

class NoOperation
  def charge(creditcard)
    "No evil operation registered"
  end

  def has_discount?
    false
  end

  def price
    0
  end
end

class Spectre
  include ActiveModel::Model
  attr_accessor :credit_card, :evil_operation

  def charge
    evil_operation.charge(credit_card)
  end

  def has_discount?
    evil_operation.has_discount?
  end

  def price
    evil_operation.price
  end

  private

  def evil_operation 
    @evil_operation || NoOperation.new
  end
end

class EvilOperation
  include ActiveModel::Model
  attr_accessor :discount, :price

  def has_discount?
    discount
  end

  def charge(credit_card)
    credit_card.charge(price)
  end
end

I guess this example is simple enough to see right away how elegant null objects can be. In our case, we have a NoOperation null class that knows how to deal with a non-existing evil operation via:

• Handling charges of 0 amounts.


• Knowing that non-existing operations also have no discount.


• Returning a little informative error string when the card is charged.

We created this new class that deals with nothingness, an object that handles the absence of stuff, and swap it into the old class if nil shows up. The API is the key here because if it matches that of the original class you can swap in seamlessly the null object with the return values we need. That’s exactly what we did in the private method Spectre#evil_operation. If we have the evil_operation object, we need we use that; if not, we use our nil chameleon that knows how to deal with these messages, so evil_operation will never return nil anymore. Duck typing at its best.

Our problematic conditional logic is now wrapped in one place, and our null object is in charge of the behaviour Spectre is looking for. DRY! We kinda rebuilt the same interface from the original object that nil couldn’t handle. Remember, nil does a poor job at receiving messages—nobody home, always! From now on, it’s just telling objects what to do without asking them first for their “permission”. What’s also cool is that there was no need to touch the EvilOperation class at all.

Last but not least, I got rid of the check if an evil operation is present in Spectre#has_discount?. No need to make sure that an operation exists in order to get the discount. As a result of the null object, the Spectre class is much slimmer and does not share other classes’ responsibilities as much.

A good guideline to take away from this is to not check objects if they are inclined to do something. Command them like a drill sergeant. As is so often the case, it’s probably not cool in real life, but it’s good advice for Object-Oriented Programming. Now give me 20!

In general, all the benefits from using Polymorphism instead of case statements apply for Null Objects as well. After all. it’s just a special case of case statements. The same goes for drawbacks:

• Understanding the implementation and behaviour can become tricky because code is spread around and null objects are not super explicit about their existence.


• Adding new behaviour might need to be kept in sync between null objects and their counterparts.

Data Class

Let’s close this article with something light. This is a smell because it’s a class that has no behaviour except getting and setting its data—it’s basically nothing more than a data container without extra methods that do something with that data. That makes them passive in nature because this data is kept there to be used by other classes. And we know already that this is not an ideal scenario because it screams feature envy. Generally speaking, we want to have classes that have state—meaning data they take care of—as well as behaviour via methods that can act on that data without much hindrance or snooping into other classes.

You can start refactoring classes like these by possibly extracting the behavior from other classes that act on the data into your data class. By doing that, you might slowly attract useful behaviour for that data and give it a proper home. It’s totally fine if these classes grow behaviour over time. Sometimes you can move a whole method over easily, and sometimes you need to extract parts of a bigger method first and then extract it into the data class. When in doubt, if you can reduce the access other classes have on your data class, you should definitely go for it and move that behaviour over. Your goal should be to retire at some point the getters and setters that gave other objects access to that data. As a result, you will have lower coupling between classes—which is always a win!

Closing Thoughts

See, it wasn’t all that complicated! There’s a lot of fancy lingo and complicated sounding techniques around code smells, but hopefully you realized that smells and their refactorings also share a couple of traits that are limited in number. Code smells will never go away or become irrelevant—at least not until our bodies are enhanced by AIs who let us write the kind of quality code we’re light years away from at the moment.

Over the course of the last three articles, I’ve presented to you a good chunk of the most important and most common code smell scenarios you will bump into during your object-oriented programming career. I think this was a good introduction for newbies to this topic, and I hope the code examples were verbose enough to easily follow the thread.

Once you have figured out these principles for designing quality code, you’ll pick up new smells and their refactorings in no time. If you have made it so far and feel you didn’t loose the big picture, I think you’re ready to approach boss level OOP status.