What to return from a Ruby method?

Let's say we are modelling London's Boris Bikes system in Ruby.

Here's a typical method that we might have in our DockingStation class (N.B. @bikes is an array):

def dock(bike)  
  fail 'Docking station full' if full?

  @bikes << bike

Seems harmless enough right? But there is a subtle problem.

The command-query separation principle states that a method should either perform an action or return a value to the caller, but not both. dock is a command, so it's easy to overlook that it also returns a value. Because all Ruby methods return a value. In this case dock implicitly returns the result of the last line evaluated in the method: @bikes << bike.

In a Ruby array, the shovel method (<<) returns the array on which it is called. So in our case, @bikes << bike evaluates to @bikes. So dock implicitly returns @bikes.

Uh oh. We've just exposed the internal implementation of our bikes collection. He he, I can now fill up my docking station with elephants (N.B. docking_station is an instance of DockingStation):

bikes = docking_station.dock(Bike.new)  
100.times do  
  bikes << 'Elephant!'

To prevent this, we need to ensure a different value is returned from the dock method. But which value? There are four reasonable candidates:

  • nil because this is a command method and the return value should be irrelevant.
  • true to convey that the bike was successfully docked.
  • bike as this is the object that was docked.
  • self which enables method chaining and might be considered a reasonable default.

While nil seems like a reasonable candidate (as it's as close as you can get to void in Ruby), on closer consideration, nil is falsey, so the following naive conditional would fail:

if docking_station.dock(Bike.new)  
  # some action conditional on successfully docking a bike

Of course, the above is a code smell, but it's entirely possible.

For the reason stated above, true might be a better choice. Indeed, it seems helpful to convey that the dock was successful. However, this introduces another subtle design smell. In the case that a bike cannot be docked (e.g. the docking station is full), an exception is raised. In the case that a bike can be docked, we return true. This is inconsistent.

This introduces another ideological debate about whether raising an exception in this case is also a design smell. Preventing bikes from being docked into a full docking station is domain logic and therefore not exceptional. Whichever argument you subscribe to, it's clear that a method which returns true, should conversely return false. Personally, I'm happy that dock raises an exception when the station is full. In the domain, docking a bike into a full docking station is not defined, so I do consider it exceptional. However it is only acceptable if we also provide an opportunity to avoid the exception by testing the docking station first; which we can do with full?.

That leaves us with bike and self.

The argument in favour of bike is twofold:

  • This is the default behaviour of attr_accessor (or attr_writer) in Ruby. When calling an attribute setter in Ruby, the method returns the new value of the attribute (which is the passed parameter). So for example, in IRB (given a class Test with a name attribute):
> test = Test.new
=> #<Test:...>
> test.name = 'Ben'
=> "Ben"
> test.name = :some_value
=> :some_value
  • It provides some convenience in our tests.

instead of:

bike = Bike.new  
docking_station.dock bike  
expect(docking_station).to include bike  

we can have:

bike = docking_station.dock Bike.new  
expect(docking_station).to include bike  

The argument against appeals to methods with multiple arguments; which one should be returned? Perhaps all of them should be? But that's getting messy.

Finally, we could return self. This allows us to chain methods for convenience:


I'm undecided which of bike or self I prefer. Clearly, any of the four options is better than returning the internal array, but I'll leave the choice of which to you.