In general I really like composition over inheritance in my Ruby code. Yes, there are times when inheritance is cool, but in my mind if you’re going to use inheritance you need to really nail it, whereas composition has much more room for error and is easier to adapt when those errors do happen. So, I wanted to show an example today of a kind of code that I recently refactored.
Here’s a somewhat condensed and exaggerated example of some patterns in some code that I saw recently at work that I wanted to get rid of.
Now, that code all works. It’ll totally do what we want it to do, and I see the idea that we’re trying to get across - we’re trying to define a common interface for updating objects, and enforcing that by calling methods in the superclass (
UpdateObject) that need to be implemented in the subclasses (
UpdateObjectB). My problem with that, though, is that it assumes we’ve correctly identified a pattern that is consistent - and will always be consistent - across how these objects are updated. If we eventually need to change that interface for one object and not the other, it starts to get tricky. As folks say, our abstractions start to get leaky. I don’t know about you, but my ability to see into the future is rather limited, and I typically bet against myself when it comes to that, so when I write code I go for maximum flexibility and maintainability, which is why I wouldn’t reach for inheritance here to do that interface definition.
But before I get to the final refactor, I wanted to address a couple specific gripes I have with the code here. First off, our superclass -
UpdateObject - bothers me. One rule I like to try and adhere to is that I shouldn’t have to look outside the file I’m in to get information I need about how a class behaves. In the case of our
UpdateObject class, there is no definition for
update_object, and also an
attr_reader for an
@object variable that isn’t acctually set in that class. What’s going on?!
Here’s how I’d refactor that class before doing the bigger refactor that I want to do:
That object is still pretty much non-functional, but at least it tells the user what it needs to do, and it also will raise helpful errors rather than just a
NoMethodError if you forget to implement the method in the child class. The biggest thing for me, though, is that if someone is reading this file that has no idea about any of the other classes that inherit from it, they won’t be so confused.
But that leads us towards the end goal that I had, which was to replace that inheritace with composition. So, let’s do that now! Let’s start off with our
If we look at it, our
UpdateObject class itself didn’t actually use the
params that we were passing to it. We just wanted to say that every object that needs to be updated needs params - but I disagree! That’s overly prescriptive and doesn’t give us flexibilty to change things later on. I think our
UpdateObject class can be simpler:
To me, that’s super clear what is going on, and the class is actually functional. It does stuff that we can test, whereas before we couldn’t actually test our
UpdateObject class - we could only test the subclasses of that class. Also, it still sets a common interface that we want to have for our objects to be updated - but with more flexibility. For example, if we needed to add some sort of action to do after the object is updated, but only some of those objects needed it, we could do something like this:
But in there we have these
collaborator objects - what would they look like? Here’s an example of one:
That object also has a clear set of behaviors that are all testable, and we can in fact create a shared example group for all of these so that we’re sure that they implement the methods they need. But we can also easily test the behavior of this object specifically, and there is no need for the tight coupling that we had with the inheritance.
So, that’s the refactoring! Personally I think this is a HUGE win, but don’t think that I’m totally against inheritance! In face, I have another post coming up soon where I show a refactoring I did that extracted a superclass and added inheritance - but I’ll also explain why that was the right choice in that (rare) case.