Untangle Spaghetti Code With a Simple Change of Perspective

That giant mess of if statements keeps staring you in the face. You feel like you should be able to simplify it, except for that Business Logic that keeps getting in the way.

For example, say you have a sales platform where you build Quotes, which have many LineItems. Except, you can have a quote with duplicate line items if they’re ads, but if you have multiple websites, you have to sum the prices together and have it show up as a single line item. Oh and also, if you buy a website and already have five ads in your quote, you have to give them a 20% discount on the website.

I can hear you throwing your laptop through the window from all the way over here.

You could write a bunch of if statements to handle these rules:

class Quote
  attr_accessor :line_items
 
  ... 
 
  def add_line_item(line_item)
    if line_item.kind_of?(Ad)
      self.line_items << line_item
    elsif line_item.kind_of?(Website)
      if @line_items.select {|item| item.kind_of?(Ad) }.length >= 5
        # TODO: Put the fractions of a cent into a bank account
        # I have set up
        line_item.price *= 0.8
      end
      existing_website = self.line_items.detect { |item| item.kind_of?(Website) }
      if existing_website
        existing_website.price += line_item.price
      else
        self.line_items << line_item
      end
    end
  end
end

But I think we can agree that that’s just terrible. How can you possibly untangle something like that?

You could decompose the method into a bunch of smaller methods, but that’s like shoving all your toys in your closet so your mom thinks you cleaned your room. And those kind_of?s would still bother me a lot.

But what if you started seeing things from the line item’s perspective, instead of the quote’s? If instead of asking what kind of line item you’re dealing with and adding it to the quote, you just told the line item to add itself to the quote?

Reverse your methods!

One of my favorite ways to refactor code is to try reversing the caller and the callee. Here’s an example, using the code above:

app/models/quote.rb
class Quote
  ...
  def add_line_item(line_item)
    line_item.add_to_quote(self)
  end
end
app/models/line_item.rb
class Ad < LineItem
  ...
  def add_to_quote(quote)
    quote.line_items << self
  end 
end
app/models/website.rb
class Website < LineItem
  def add_to_quote(quote)
    if quote.line_items.select {|item| item.kind_of?(Ad) }.length >= 5
      # TODO: Put the fractions of a cent into a bank account
      # I have set up
      self.price *= 0.8
    end
    existing_website = quote.line_items.detect { |item| item.kind_of?(Website) }
    if existing_website
      existing_website.price += self.price
    else
	  quote.line_items << self
    end
  end
end

It’s not perfect. website.rb still needs a lot of refactoring help, and I’m not happy with how reversing the methods broke encapsulation of line_items.

But you’ve removed the first layer of complexity. You can now put code on the LineItem or the Quote, depending on where it makes the most sense. The LineItem objects can use inheritance and mixins to handle similarities and differences between each LineItem subclass. Plus, it’s now really easy to add new LineItem subclasses without bloating your add_line_item method.

Your code is a little cleaner, and a lot more flexible. So generally, I’d call it a win.

Where you might not want to use this pattern

As useful as Reversing Method is, there are some reasons you might not want to use this pattern:

  • It can break encapsulation. You might have to expose attributes on the Quote object that you didn’t want to expose publicly.

  • It can increase coupling. Both Quote and Ad now need to know about each other. And depending on how much they need to know about each other, it can make your code more complicated.

  • It can violate the Single Responsibility Principle on Ad, because now Ad has the responsibility of knowing how to add itself to a Quote.

You can usually work around these problems. But you should be aware of them, because you don’t want refactoring to make your code worse!

Why it’s one of my favorites

Even with those problems, this is one of my favorite refactorings. The code I write after using this pattern tends to be clearer and more confident.

But even when it isn’t, using this pattern makes me think about the relationships between my objects in a different way. When I get into the “this feature is awful, I can’t believe I have to write this terrible code to handle it” rut, it kicks my brain into seeing new ways I can solve those problems. It forces me to think about how I could structure my code differently, and that’s incredibly useful.

Give it a try in your own code

Like many of my favorite patterns, I first came across Reversing Method in Smalltalk Best Practice Patterns, and it’s been a valuable tool ever since.

Next time you have a hard time dealing with similar objects that have slightly different behavior, give it a try! If you like the new code better, keep it. Even if you don’t, though, it’ll take your mind down a path that will lead you to better code.

Pushing through tutorials, and still not learning anything?

Have you slogged through the same guide three times and still don't know how to build a real app?

In this free 7-day Rails course, you'll learn specific steps to start your own Rails apps — without giving up, and without being overwhelmed.

You'll also discover the fastest way to learn new Rails features with your 32-page sample of Practicing Rails: Learn Rails Without Being Overwhelmed.

Sign up below to get started:

Powered by ConvertKit

Did you like this article? You should read these:

Comments