Skip to content
Holger Frohloff

Quick wins, part 3: Keep it local

1 min read

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:

1class Foo
2 def initialize(bar_service)
3 @bar_service = bar_service
4 end
6 def quux
7 if @bar_service.greeting == "hello"
8 @bar_service.greet("goodbye")
9 else
10 @bar_service.greet("hello")
11 end
12 end
15class BarService
16 attr_accessor :greeting
17 def greet(message)
18 @greeting = message
19 end

What bothers me with this code? The method quux has too much knowledge about how the @bar_service works. 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 greeting and 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 greeting and greet work inside the BarService. Start with the conditional, like this:

1class Foo
2 def initialize(bar_service)
3 @bar_service = bar_service
4 end
6 def quux
7 @bar_service.greet
8 end
11class BarService
12 attr_accessor :greeting
13 def greet
14 if @greeting == "hello"
15 @greeting = "goodbye"
16 else
17 @greeting = "hello"
18 end
19 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.

© 2022 by Holger Frohloff. All rights reserved.
Theme by LekoArts