Just because the code looks the same

Just because the code looks the same

Head First Design Patterns. It’s been a long 3 and a half years since I read that book and one principle that always springs to mind is “encapsulate what varies”. Help me very much it did as a self-taught code monkey on the way to DRY enlightenment.

Two big principles there that I’m shortly about to explain why you should totally ignore them.

The Bait

Have a look at the two classes below. They are handlers for a typical web framework, although it doesn’t matter which here. See if you can spot a pattern, or an “encapsulate what varies” opportunity.

https://gist.github.com/3914583.js?file=gistfile1.cs

Of course you can see it, pretty simple right? Those two Get methods have identical flow of logic. All that’s different is the types used and the name of variables. Imagine if we had a third handler for Kazakhstan jobs, we could create it a lot quicker if we had encapsulated what varied here into an abstraction.Adding the abstraction

Here she is, 

https://gist.github.com/3914607.js?file=gistfile1.cs

Adding a new handler is quite easy now should we need it. We just use the generic jobs finder. Plus, because the logic is encapsulated we can change it for all three at once — that must save timeLosing my cool

I haven’t explained the cost of that abstraction, but let’s take a quick rain check. How many lines of code were there in the first place? There were 6 lines each in the two handlers with another similar one on the way. Doesn’t sound like we’ve the potential to reduce much complexity, but read on if you’re undecided.

So if I want to understand what goes on, no longer is the logic all laid in six sequential lines of code. Now we have the new JobsFinder. Problems with this are:

  • I have additional navigations to work out what is going on
  • I have to create a common interface for all the specific job finders
  • I have to create a common interface for the view model
  • Have to rename the view model properties to more general terms (lost expressiveness)
  • Generics and constraints always make the code nice and pretty
  • All this stuff takes extra time and actually increases complexity When to refactor

When it’s only 6 lines of code I’m quite prepared to duplicate that logic up to 5 times, probably more. What takes the most time is revising the acceptance tests. Tweaking the code in multiple places is absolutely nothing.

There is also another new problem — different handlers may need change for different reasons. We thought we encapsulated a concept universal to all job handlers, but it turns out each country wants to go in its own direction and all we simply removed duplicate code and not concept.

When one handler does change independently, suddenly there is an opportunity to force that onto the existing abstraction. One day you wake up and it’s a Swiss army knife handling all sorts of edge cases (seen it happen).

In a simple case like this, I would wait till I was so annoyed at having to keep making the same slight change to 10+ handlers because they always kept changing for the same reasons together. And then it’s still a trade off.

Just because the code looks the same, doesn’t mean you need to invent abstractions.