Quick Wins, Part 3: Keep It Local
Yesterday I closed with this idea:
Spot places where knowledge about something does not belong.
What do I mean by that? Sometimes I come across some code that does not read right. I will use a pseudo code example to illustrate:
class Foo def initialize(bar_service) @bar_service = bar_service end def quux if @bar_service.greeting == "hello" @bar_service.greet("goodbye") else @bar_service.greet("hello") end end end class BarService attr_accessor :greeting def greet(message) @greeting = message end end
What bothers me with this code? The method
quux has too much knowledge about how the
Foo.quux knows that the
@bar_service has an instance variable called
greeting and at least one specific value it might have (
"hello"). It also knows two values that the
greet() method might be called with.
Now it happens that this knowledge about how the
greet work, is also spread into other parts of the application. What happens if you need to change something about the
greet() method? You have to find every place in your application and update it to reflect the new changes.
This isn’t good.
There are places like this inside many applications. You might need some practice to spot them, but with some practice it becomes easier. For this example I would like to suggest to move all knowledge about how
greet work inside the
BarService. Start with the conditional, like this:
class Foo def initialize(bar_service) @bar_service = bar_service end def quux @bar_service.greet end end class BarService attr_accessor :greeting def greet if @greeting == "hello" @greeting = "goodbye" else @greeting = "hello" end end end
Now we are free to change the internals of the
greet method. We could add a third option or change it completely. The class
Foo does not need to change at all. It continues to call
greet as if nothing has happened.
One of my overarching topics is testing. A refactoring like this should be covered with tests. Not only do you need tests for the
Foo class, but also for how
BarService.greet works. And for every part of the app that interacts with either.
Tomorrow we’ll look at another way to do a refactoring.