This is a shit tutorial

This is a shit tutorial
Those who can't teach. Which is utter bollocks because teaching is a very different skill to doing

I just went back and re-read some of the earlier articles in this series. I'm sat here looking at the code I've written and there's a ton of stuff I wrote that in those articles that simply doesn't match how the actual implementation turned out.

So if you're here expecting a tutorial in how to write "clean code" in Rails, I'm afraid I'm letting you down. Don't go away though - I'm really honoured you're here and I promise you'll that what we're going through is really valuable.

The key thing here is building a new app is a discovery process. As new bits get added decisions we made earlier turn out to be less than optimal. Normally, we just put up with this - we've got deadlines and 🤷🏽‍♀️ things always get messy, right? But, the point of this exercise - doing things "not the Rails Way" - is to make it easy to go back and change our minds. We have no excuse for letting the shape of the code gradually degrade, deadlines or no deadlines.

Things I've learnt

The Single Responsibility Principle

The Single Responsibility Principle is the "S" in SOLID. But now I believe I've had it wrong all these years. And judging by the code I've seen, written by people who claim to be following SOLID principles, I believe most people have got it wrong as well.

The SRP is generally referred to as "a module should have one, and only one, reason to change". This is interpreted as "a module should do one thing and one thing only". And the net result is lots and lots of tiny little classes, often with a single method that does its one thing, and a stupid long name. Naming gets really difficult because there are so many other tiny classes knocking around, all doing very similar things. These types of app might be great while you're writing them but they become impossible for newcomers to figure out what's going on. Each individual class is simple but how they interact and the effects they have on each other is totally opaque.

But the definition of the SRP I mentioned earlier is wrong. The actual definition is "a module should be responsible to one, and only one, actor". But what is an "actor"?

Bob Martin gives the example of an Employee class that has methods to calculate the hours worked over the last month and the wages due for the last month.

If we follow the incorrect definition of the SRP, we might say "the employee class does two things and it should only do a single thing - so what we need is an Employee::HoursCalculator and an Employees::WagesCalculator". But this is wrong. Or it's right for the wrong reasons, and those wrong reasons have led us to a mistake.

The problem with the Employee class is not that it's doing two things. Actually, the Employee class could do ten things and it wouldn't matter. The problem is that is has two owners.

The hours_worked_during(month) method is there because the HR department needs to ensure that the employee complies with any employment laws. But the wages_due_for(month) is there because the finance department needs to run payroll at the end of the month.

If we split the class into Employee::HoursCalculator and Employee::WagesCalculator we are splitting the responsibilities but we are doing it by accident, not on purpose. And that is likely to lead to some dangerous mistakes.

For example, if our employee is paid hourly, we might assume that Employee::WagesCalculator can call the Employee::HoursCalculator and multiply the result by the employee's rate. That's code reuse - and that's a good thing isn't it?

But what if there's a change in employment law (a very common occurrence) and the number of breaks the employee must take during a working day changes. That means, for a given 9-5 working day, the hours worked changes - from the point of view of the HR department. But it's still a 9-5 working day from the point of view of the finance department. And don't forget that perspective is really important when working in software.

The SRP actually says we should actually split the Employee class into an HRDepartment::HoursCalculator and FinanceDepartment::WagesCalculator. The distinction might look small but these details are very important.

As mentioned above, our employee is paid hourly and we're in the process of writing the FinanceDepartment::WagesCalculator. We realise that we need to calculate the number of hours worked in the previous month. We scan the codebase and find the HRDepartment::HoursCalculator. Perfect - we can just reuse that, can't we?

But the fact that HRDepartment::HoursCalculator is in the HRDepartment namespace should make us stop and think.

Because that code is not our responsiblity. It belongs to the HR department.

If we use it, we're violating the Single Responsibility Principle because that code now becomes the responsibility of two different owners - each with different reasons to change the definition of how many hours have been worked.

The correct solution is to build a third class - FinanceDepartment::HoursCalculator - which is only responsible to the finance department. Employment law can change all it wants, but finance will be unaffected[1].

Type checking is not all bad

Years ago I used to work with statically-typed compiled languages. I find ruby a total breath of fresh air - mainly because I can just start writing code instead of having to write a ton of boiler-plate type definitions.

Ruby is like writing in English
Python is like writing in Maths
Statically-typed languages are like submitting a planning application to your local council

But, building an app in this style, where the core logic is unaware of the edge implementations, does feel a bit scary. We're pulling random objects and classes out of the locator, so how can we be sure those objects will actually do what we want?

In static languages, we would define interfaces. These are an "outline" of a class - detailing the methods and parameters that the class has, but without an implementation. So our core classes expect a particular interface and our edge classes implement those interfaces. And the compiler enforces the rules.

To achieve the same effect in ruby, I've been using dry-types - in particular Types.Interface. This is not compile-time checking - we're using ruby after all. I simulate compile-time checking by using guard to run the relevant RSpec file every time I hit "save"[2]. But it's still not as strict or comprehensive as static type-checking.

However, having thought about it for a while, I think I prefer this pseudo-interface to the alternative.

In static languages, when defining our interfaces, we have to give each one a name. That name is used by the core class to cast the object received from the locator[3]. And that name is used by the edge class to prove to the compiler that this object fits the bill. If we look at some of the dry-type interfaces I've defined, we'll see that they are tiny. Our core classes only expect the edge classes to implement one or two methods. But different core classes are going to need a different one or two methods from the edge classes. So the number of interfaces explodes and choosing names for those gets harder and harder.

In, for example, dart, the FileSystemLinkFolder class might require a LinkCreator interface, where LinkCreator defines a single function createLink(Linkable source), which in turn requires an interface (or maybe class) called Linkable that needs defining somewhere as well.

With dry-types in ruby, we do not need to give our interfaces names. The FileSystem::LinkFolder class requires an object that matches Types.Interface(:create_link_to). The edge class [4] only needs to implement a method create_link_to(source)[5], with no requirement for any magic names or definitions.

Back to our scheduled programme

Anyway, as I said earlier, this is a shit tutorial.

I've changed my mind many times, skipped over the details of what the code looks like and I've not even given you access to a repo so you can follow along at home.

Because, actually, my code doesn't matter. It will change further as we progress and I'll discover I was wrong about things and have to go back and do stuff again.

What does matter is that I explain why we might need those changes, how we make those changes easy to implement and, most importantly, that we lay down the principles that we're going to work to.

  • at the start of the project we don't know very much, so we leave our decisions till as late as possible, when, hopefully, we will know more
  • we define what the application will do in core classes that are implementation independent
  • we put all the implementation details into edge classes that actually do the dirty work - talking to the database, shoving pixels on-screen, sending and receiving over the network
  • we have "feature specifications" that allow us to set boundaries on our work - meaning we do the bare minimum to satisfy the user's needs
  • we have executable feature specifications that drive the system from the outside in[6], proving that the system actually does what the user needs
  • we have "unit specifications" that force us to keep our code testable[7], which in turn means we have the flexibility and confidence to change our minds and rewrite things
  • we write our unit specifications in advance so that we can ask ourselves "how would I like this tiny piece of the API to work in an ideal world?" rather than getting bogged down in complex technical details

Anyway, I've made some decisions.

My original plan was to build an API and a mobile app that uses that API. But my clients need to see some results soon (those deadlines again) and my mobile dev skills aren't as sharp as my web skills. So I'm going to start building an HTML front-end and we'll be using the same spec-first approach there as well.

Once again, thank you so much for reading and giving me your time. There will be some actual code for you to look at next time, I promise.

  1. It might be that these two HoursCalculators have some shared code - probably in a third module. But this shared code needs to be independent of HR and Finance and must have no reason to change if and when employment or tax laws change. ↩︎

  2. Don't forget - these specs are fast because we're independent of the database. ↩︎

  3. This is actually a run-time check, so even though we have a compiler, it could still fail when the app is running. ↩︎

  4. In this case, an ActiveRecord::Model called Folder ↩︎

  5. OK - we don't mandate the parameters in our interface - but the simplicity we gain is worth it in my opinion. ↩︎

  6. From web request through to the database and back again in this particular app ↩︎

  7. Which is just another way of saying our code is nicely decoupled ↩︎

Rahoul Baruah

Rahoul Baruah

Rubyist since 1.8.6. Freelancer since 2007, dedicated to building incredible, low-cost, bespoke software for tiny businesses. Also CTO at Collabor8Online.
Leeds, England