Dynamic versus Static types (or ruby against the world)

Where we're at

As I mentioned last time, the code in front of me doesn't really match the code I was describing as I was writing the previous articles. That's because, as I progressed, I came across problems, overcame those problems and changed the code to match. The requirements as I understand them now are different to the requirements as I understood them then.

This is normal and good^[1] - remember, the start of the project is when you know the least about how it's going to work.

Currently, I have a core StandardProcedure class which acts as my object-locator as well as my events-publisher. This class lives in lib/standard_procedure_app/standard_procedure.rb and is configured in a Rails initialiser. The instance is then stored in Rails.application.configuration.standard_procedure for any other code to reference^[2]. Within the specs, this StandardProcedure object is created and configured in a before clause, then passed to the objects being tested, so each test has its own isolated instance to work from.

I also have, in lib/standard_procedure_app a series of "features". Each of these consists of a module (for example lib/standard_procedure_app/file_system.rb) with classes representing the features (or use-cases) within that name-space (such as lib/standard_procedure_app/file_system/link_folder.rb). The feature implementations are sub-classes of StandardProcedure::Feature or StandardProcedure::AuditableFeature, two base classes that handle permissions, events and auditing (in lib/standard_procedure_app/standard_procedure/ as they are central to how the system works).

None of the stuff mentioned so far is Rails specific - it's all pure business logic. To enforce that, all the specs for these classes (in spec/lib/standard_procedure_app) reference spec_helper, rather than rails_helper. The latter loads the entire Rails environment, whereas the former loads nothing but RSpec. This means we need to add explicit require statements when any source files have references to other source files. That enforces very explicit dependencies within the "feature" classes and we can make sure they stay clean and independent of the rest of the system.

Then, I have some normal Rails stuff. In app/models I have my ActiveRecord classes. In app/controllers I have some controllers and base controllers.

And in app/features I have my app-specific features.

The "core" features, described above, are pure logic, defining the behaviour of the application in the general case.

The "app" features are specific implementations of tasks that users will need to perform, plus the extra stuff isn't tied to the core logic.

So I have app/features/user_sign_in and app/features/dashboard - handling logging in and viewing your dashboard respectively. The dashboard isn't a "core" feature of the system. You can easily imagine two different client apps, both using the same system configuration but displaying wildly different dashboards.

Within an "app" feature, I currently have controllers, presenters and components.

A controller is, as you would expect, a Rails controller that handles incoming web-requests and decides how to return a web-response.

The presenter packages up the data (usually by using the object-locator and then calling "core" features) and give it back to the controller.

And the components generate the HTML which is shown to the client.

In a standard Rails app, these would be views, but I'm using Phlex components. I like that they're self-contained ruby files and I can organise them by feature - for example, I have a UI namespace for general components (Ui::Card, Ui::Icon and Ui::FlexPanel) and feature specific namespaces, (like Dashboard::Sidebar and FileSystem::FolderPage).

To be totally honest, I'm not sure about the presenters.

I put them in to make it easy to test that the correct data was assembled by the controller and passed to the components. But I think they might just be unnecessary and I could just write controller specs instead. If the same package of data was needed by two different controllers (or maybe by something else, like a rake task), then they would be useful, but, so far, I've not had a need for that. Which means I'll probably remove them - the combination of controller to package the data and component to transform it into a display format is probably enough.

So, in terms of the current code-base, that's where we're at.

I think the next article is going to be about how these "app-specific" features are put together - and it will probably include an exploration of whether these presenters are needed or not. Because the best way to have fewer bugs is to have less code.

But today, I want to talk about types and type-checking.

How using doubles improves your code

I mentioned before about how we're using doubles in our specifications.

Some people frown on this, because you're not testing what the system is actually doing. Which, in turn, means that your tests can be brittle. If a class that we depend on changes (say a method name), the test will still pass as it's blindly using a double instead of the real class.

That's why we have the "full-stack" specifications.

Not only are these written from the user's perspective, proving that the system achieves the features they need, these specifications also push a request end-to-end. They start with a web request that travels through the code all the way to the database and back again. It has to be said, though, that these feature-specifications are not as exhaustive as the unit-specifications, so there's still a chance mistakes will slip through the net.

But the point of writing unit-specifications isn't just to prove that the unit (class or object) in question works correctly. It serves two other, arguably more important purposes.

Firstly, they act as a source of live, always up-to-date, documentation on how the unit is expected to be used. Can't get something to work? Refer to the specification and look at some real examples (without tons of boiler-plate code to set up the test harness).

Secondly, they are a design tool. By putting these doubles in place, we are driving the design of the system, mapping out the dependencies and setting expectations^[3]. And, if we find that our doubles are getting hard to configure, that is the code telling us that there is a simplification we can make.

Let's look at an example.

For my retailer client, they want their staff to be assigned a shift and, when they arrive at work, they "check-in". If they check-in on time then all is good, but if they check-in late then the manager should be notified. In Standard Procedure, this notification process will be modelled by a configurable Workflow.

So I added in a feature, called WorkingTime::CheckIn that handles the first part. Then, in my "Recording a timecard" specification, one of the steps configures an observer that watches for working_time/check_in.completed events. This event includes the Shift that the staff member has been assigned and the TimeCard that the check-in process has just created for them. The observer passes those details to a WorkingTime::DiscrepancyCheck that examines the Shift and TimeCard and decides what to do with them.

As an aside, note how the act of checking-in and the act of deciding if this is a discrepancy are totally decoupled. The only link between them is the observer that watches for events. This gives us great flexibility in the future. I know that another of my clients, a child-care agency, also needs their staff to check-in and out of shifts but they use a totally different process for dealing with time-cards. This decoupling means I can use the same WorkingTime::CheckIn feature for both systems but handle time-card management in a totally different way by changing the configuration of the observer.

This is my first attempt at the unit specification for the WorkingTime::DiscrepancyCheck. We set up some doubles for the user, shift, time card and add in a reference to a new feature, called Workflows::StartWorkflow. Then we set some expectations on the user, to ensure they have permission to do this work, and try and configure our workflow doubles.

  it "starts the discrepancies workflow if the shift was started late" do
      @user = double "user"
      @shift = double "shift", starts_at: 2.minutes.ago
      @time_card = double "time_card", starts_at: 1.minute.ago
      @start_workflow_feature = double "workflows/start_workflow_feature"
      @start_workflow = double "start_workflow"
      @standard_procedure.register "workflows/start_workflow", @start_workflow_feature
      @card = double "card"

      allow(@user).to receive(:can?).with(:check_in_to, @shift).and_return(true)
      expect(@start_workflow_feature).to receive(:new).with(app: @standard_procedure, user: @user, auditor: @auditor, context: anything, item: @time_card).and_return(@start_workflow)
      expect(@start_workflow).to receive(:call).and_return(@card)

      WorkingTime::DiscrepancyCheck.new(app: @standard_procedure, user: @user, auditor: @auditor, context: nil, shift: @shift, time_card: @time_card).call
  end

This looks quite messy to me. We create a double of the start_workflow feature class, then create a double of the start_workflow instance, then have to set two expectations - one for new and another for call.

If we were using "real" objects, we probably wouldn't even notice this^[4], but looking at it using doubles, it feels clunky. The specification is telling us that our discrepancy_check needs to know about a class, not an object, registered within the locator. And then it needs to know how to instantiate that class, as well as how to handle the resulting object the instantiation creates.

It feels to me like the discrepancy_check needs to know too much about start_workflow.

However, life is what it is, so the most important thing is to get something working first. This is the implementation of WorkingTime::DiscrepancyCheck I wrote.

require_relative "../standard_procedure/auditable_feature"

module WorkingTime
  class DiscrepancyCheck < StandardProcedure::AuditableFeature
    attribute :shift, Types.Interface(:starts_at)
    attribute :time_card, Types.Interface(:starts_at)

    def authorised?
      user.can? :check_in_to, shift
    end

    def perform
      add_to_discrepancies(:started_late) if started_late?
    end

    private

    def started_late?
      shift.starts_at < time_card.starts_at
    end

    def add_to_discrepancies(stage)
      start_workflow_feature.new(app: app, user: user, auditor: auditor, context: context, stage: stage, item: time_card).call
    end

    def start_workflow_feature
      app["workflows/start_workflow"]
    end
  end
end

The specification now passes and we can move on to figuring out how Workflows::StartWorkflow is supposed to work.

However, that feeling is sitting with me and I know that as I implement more and more of these features, the same issue is going to crop up again.

So, now we have something that works, can we go back and make it a bit better?

Let's put ourselves into the perfect world. How would we like it to look? How about this?

  it "starts the discrepancies workflow if the shift was started late" do
      @user = double "user"
      @shift = double "shift", start_at: 2.minutes.ago
      @time_card = double "time_card", starts_at: 1.minute.ago
      @start_workflow = double "start_workflow"
      @standard_procedure.register "workflows/start_workflow", @start_workflow
      @card = double "card"

      allow(@user).to receive(:can?).with(:check_in_to, @shift).and_return(true)
      expect(@start_workflow).to receive(:call).with(app: @standard_procedure, user: @user, auditor: @auditor, context: anything, item: @time_card).and_return(@card)

      WorkingTime::DiscrepancyCheck.new(app: @standard_procedure, user: @user, auditor: @auditor, context: nil, shift: @shift, time_card: @time_card).call
  end

We've removed the reference to the start_workflow class, so there's no need to call new. Then we moved the parameters we're using into call. This makes setting up our doubles a little bit simpler because the dependency_check requires less knowledge about start_workflow.

This dodgy feeling about the shape of the code didn't come out of nowhere. Another aspect of what I'm uncertain about is to do with type-checking.

Each of our features uses dry-types' Types.Interface(:method_name) construct to validate its incoming parameters. When we supply a Shift to WorkingTime::DiscrepancyCheck, dry-types automatically tests that shift to ensure it has a starts_at method. This makes our expectations of the shift object completely explicit.

But when we pull an object out of the locator, there is no such checking. So far, I've registered a mix of Features and ActiveRecord classes in there - and in some cases, we've used specially defined class methods on those ActiveRecord classes.

But what if, during the configuration phase, we register a class or object that does not conform to the expectations of this particular feature?

In "how to halve your support costs" we registered an Auditor object into the system. That auditor must be able to respond to record_authorisation_failure, start_activity, record_completion_for and record_failure_for. That's a hefty list of dependencies that the auditor must fulfil.

So it's been bugging me - there's something not quite right about this design.

Let's attack the initial problem, the need to use new and then call, first though.

Call and the object locator

The first thing I identified was how we needed to use new followed by call to get an object from the locator to do something, which feels like a step too many.

I was going to start reimplementing my "features" using dry-transaction, which is an immutable object that builds a "functional pipeline". I want to talk about functional pipelines, because as someone who loves object-oriented programming, they are absolutely amazing, even though they come from a functional programming background^[5]. But it's too much of a digression for now, so I'll come back to that another day.

Instead of using dry-transaction, which would have required rewriting big chunks of my logic so far, the solution turned out to be incredibly simple. Everything to do with the application logic is handled by a sub-class of Feature - where Feature itself defines a framework for authorisation and publishing events.

I added the following to the Feature class:

  class << self
    def call(**)
      new(**).call
    end
  end

Now, every Feature has a class method, call. call doesn't care what parameters it gets given, it just invokes new (with those unspecified parameters) followed by call. If the caller provides the wrong parameters, new will fail, so, even though we're using ruby's ** for the parameters, it still requires the correct data to be provided. The only issue is, documentation-wise, it's much harder to see, at a glance, which parameters are required.

Return to interfaces

As I've mentioned many times in this series, I don't like statically type-checked languages. I feel like I'm spending a lot of my time trying to keep the compiler happy when I should be spending my time trying to keep my clients happy.

But some sort of type-checking is useful. Of course, ruby actually does type-checking at run-time. If you had the code @my_object.do_something but @my_object doesn't have a do_something method, then ruby will let you know.

Compiler fans will tell you that a compiler would catch that mistake at compile-time, not run-time. To make that work, you need to have defined classes or interfaces so the compiler can offer you that guarantee.

But even in compiled languages, you still have run-time checking. Every time you cast an object from one type to another the actual check happens at run-time. Or, in Dart, which promises "no null exceptions", you can still write code that allows null exceptions to happen at run-time^[6].

This is because the compiler simply cannot catch everything. Especially as your system grows in directions you didn't anticipate at the start.

However, given that everything is handled by this core object-locator (my StandardProcedure class), it would be nice to have some idea of what we're looking at when we pull something out of there.

Non-intrusive type-checking, the dynamic way

I've divided my "features" (the business logic) into folders and namespaces. We've got Authorisation and FileSystem and WorkingTime and more. Each of these namespaces contains one or more features - for example, WorkingTime has WorkingTime::CheckIn, WorkingTime::CheckOut, WorkingTime::DiscrepancyCheck for recording and managing your time-cards.

When calling WorkingTime::CheckIn you need to provide a Shift object. So, in the WorkingTime module, I've used dry-types to define an interface that specifies what a Shift needs, in these circumstances (and these circumstances alone, as we'll see later).

In books that use, say Java, as an example, I've seen this handled by defining a Shift interface which is then implemented by a ShiftImplementation class. Apart from the fact that ShiftImplementation is a terrible name, I think this approach does too much. WorkingTime::CheckIn doesn't actually care about everything a Shift can do. It only cares that this particular shift has a check_in method.

So in the WorkingTime module, I've defined an interface called WorkingTime::CanCheckIn. The WorkingTime::CheckIn feature then specifies that any shifts we give to it must match WorkingTime::CanCheckIn.

The WorkingTime module looks like this:

require "dry/types"

module WorkingTime
  include Dry.Types()

  Clock = Interface(:utc)
  Shifts = Array.of(Interface(:user, :notification_sent?, :within_notification_period?))
  CanCheckOut = Interface(:check_out)
  CanCheckIn = Interface(:check_in)
  HasStartTime = Interface(:starts_at)
  HasEndTime = Interface(:ends_at)
end

We've got a "clock" (that can convert itself to a Time in utc), the aforementioned CanCheckIn, its partner CanCheckOut and the related HasStartTime and HasEndTime interfaces. Plus we have Shifts which is a array of objects that have a certain set of methods. I'm not so sure about this last one - it's used when sending notifications to people to remind them that they have a shift later that day. I think the problem is that the name is wrong and it should be called something like Notifiables. (adds to to-do list)

The point is that each of these interfaces (apart from Clock) is currently implemented by a single Shift class (and in this case, Shift is actually an ActiveRecord model). But, Shift doesn't need to know the names CanCheckIn or CanCheckOut^[7] and the actual features (WorkingTime::CheckIn , WorkingTime::CheckOut, WorkingTime::Whatever) don't care that they're receiving Shifts^[8]. They only care that the things it's receiving are capable of checking-in or checking-out.

This is powerful.

Checking in and out may not just be a feature that relates to shifts. Maybe, at some point, we will need to add checking in or out of a training course or a board meeting or something else. So, instead of linking these capabilities to a specific name and tying our hands behind our backs before we've even started, we've sliced our interface definitions into the smallest thing possible. All of which gives us greater flexibility in future.

The WorkingTime::CheckIn code is a sub-class of Feature, which in turn is a Dry::Struct. So the type-checking is handled by Dry::Struct.

module WorkingTime
  class CheckIn < StandardProcedure::AuditableFeature
    attribute :shift, WorkingTime::CanCheckIn
    attribute :latitude, WorkingTime::String.optional
    attribute :longitude, WorkingTime::String.optional
    attribute :accuracy, WorkingTime::String.optional

    def authorised?
      user.can? :check_in_to, shift
    end

    def perform
      shift.check_in time: Time.now, latitude: latitude, longitude: longitude, accuracy: accuracy
    end
  end
end

But what about those other objects? The ones that we pull out of the object locator? As I mentioned before, the auditor has a more complex interface, with four methods that it needs to support. Let's see how that is handled.

First of all, my StandardProcedure class has its own set of "core" interfaces, in addition to the Dry::Container^[9] and Dry::Events^[10] requirements.

require "dry/container"
require "dry/events"
require "dry/types"

class StandardProcedure
  include Dry::Container::Mixin
  include Dry::Events::Publisher[:standard_procedure]
  include Dry.Types()
  require_relative "standard_procedure/errors"

  App = Interface(:publish, :register)
  Auditor = Interface(:record_authorisation_failure, :create_context_for, :record_completion_for, :record_failure_for)
  User = Interface(:can?).optional
end

You can see the definition of StandardProcedure::Auditor - with those four methods listed.

Then, when we need to use an auditor, we can test it for compliance - just as you might cast an object from one type to another in a static language.

Within StandardProcedure::AuditableFeature there is a protected method, auditor, that retrieves the correct object from the locator.

  protected 

  def auditor
    @auditor ||= StandardProcedure::Auditor[app["auditor"]]
  end

app["auditor"] grabs the object from the locator. And we wrap that call in StandardProcedure::Auditor[...]. This is how you can use dry-types to verify that the object you've supplied matches the interface defined. If it doesn't match, we get an exception.

So there is a place for type definitions after all

I've said many times, I really don't like statically typed languages^[11]. I feel like I'm wasting my time, and brain capacity, writing code that languages like ruby (or python or javascript) can infer automatically. Why should I do the work when the computer can do it for me?

But there is definite value in type-checking. If you've ever tried to understand some of the internals of Rails by navigating through the source code, it can be infuriating trying to figure out what parameters a particular method actually accepts. Stamping a simple type on to those objects makes things much easier to understand, as well as highlighting potential problems at run-time.