Code review: Rails process management

#Technic 01/22/2020

A friend asked a code review of some Rails controller.
He has couple of actions that requires to be chained, one to the next.
All of those actions can, of course go wrong.
Let's see how we cleaned this code.

Here some bad bad code that you should no be writing:

class MyController < ApplicationController
  def create
    response = fetch_api
    return unless response
    save_into_db response
    return unless response
    load_from_db response
  end
  
  protected
  
  def fetch_api
    "...SOME CODE..."
  end
  
  def save_into_db
    "...SOME CODE..."
  end
  
  def load_from_db
    "...SOME CODE..."
  end
end

First problem, the protected method should be extracted into services. This way they are reusable and easier to test.

But it’s not my friend code, instead he created something more like this.

Extraction into command class

class Command
  def self.call(*params, &block)
    new(*params, &block).call
  end
  
  def call
    reset
    run!
  rescue CommandException => e
    errors.push e
  ensure
    self
  end
  
  def reset
    self.payload = nil
    self.errors = []
  end
  
  def success?
    errors.empty?
  end
  
  def result
    payload
  end
  
  def merge(cmd)
    self.payload = cmd.result
    self.errors += cmd.errors
  end
  
  attr_reader :errors
  
  protected
  
  attr_accessor :payload
  attr_writer :errors
end

class FetchApiCmd < Command
  def run
    response = SomeApi.fetch('something')
    if response.status == 200
      self.payload = response.data
    else
      self.errors.push "invalid response #{response.status}"
    end
  rescue StandardError => e
    self.errors.push e.message
  end
end

Allowing the following version of the controller :

class MyController < ApplicationController
  def create
    @response = FetchApiCmd.call params[:something]
    return unless @response.success?
    SaveIntoDBCmd.call @response, params[:something_else]
    return unless response.success?
    @response = LoadFromDBCmd.call @response
  end
end

The methods are now separeted into *Cmd classes, stored in app/commands. And the view can deal with the @response. It’s a bit better, but we are not there yet.

Chaining command

I wrote some Gist similar to this :

class ChainCommands < Command
  def initialize(&block)
    self.cmds = [block]
  end
  
  def then(&block)
    cmds.push &block
  end
  
  def run
    run_subcmd cmds.first
    return self unless success?
    res[1..-2].each do |cmd|
      run_subcmd cmd, result
      break unless success?
    end
    self
  end
  
  protected
  
  def run_subcmd(cmd, *arg)
    merge cmd.call(*arg)
  end
  
  attr_accessor :cmds
end

Allowing the following refactor :

class MyController < ApplicationController
  def create
    @response = process.call
  end
  
  protected
  
  def process
    ChainCommands
      .call { FetchApiCmd.call params[:something] }
      .then {|last| SaveIntoDBCmd.call(last.result, params[:something_else]) }
      .then {|_last| LoadFromDBCmd.call }
  end
end

We have now a more explicite definition of the process, it’s a chain of commands, and the view handle a response object containing the errors or the final payload.

The protected method could be also be extracted as a separed command.

Challenging the result

We haven’t look at existing code that could help. It can be some gem that implement in a better and more tested way this command and chain system.

For example in the ruby toolbox

We could also have worked with Exception. Actually we talked about it, but it was not fitting the current implementation of the HTML view behind. But the question is still on, what if (and it will) an exception is triggered by one of the commands ?

Well some of them are handled by the commands (Timeout, NetworkError, Transaction …). I personnaly would prefer to handle on the top level the exception, I think the code would have been more simple. However, this command approach allow more flexibility down the road. For instance, if you want to branch on another process after a failure.

Happy code !