Code simplicity - Reading levels

When we write code, who is it for? Is it for the machine? It is the machine that will parse and run your code. Is it for the next developer? It is that person that will spend time reading and updating the code. Is it for the business? After all, the code wouldn’t exist if it had no purpose. Obviously I think code targets all of them.

The goal is to not only make your code understandable by the machine, but also to your future self and to the business itself. This isn’t an easy thing. In this article I only aim to go past the machine-readable to reach the next developer level. To do that, let’s refactor a small fictive class…

Initial code

In the previous article of the serie, I started extracting a chunk of code from a Rails’ controller to an external object. I did it in a very simple way and here is the result:

class ConfirmOrder < Command
  def initialize(order, payment_token, notify: true)
    @order = order
    @payment_token = payment_token
    @notify = notify
  end

  validate do
    payment = @order.payments.new(token: @payment_token)

    if !payment.valid?
      add_error(:payment_token_invalid)
    end

    if payment.amount != @order.sales_quote.amount || payment.currency != @order.sales_quote.currency
      add_error(:payment_amount_mismatch)
    end
  end

  perform do
    @order.payments.create!(token: @payment_token).capture!

    Order.transaction do
      invoice = @order.invoices.create!({
        amount: @order.sales_quote.amount,
        currency: @order.sales_quote.currency,
      })

      @order.sales_quote.items.find_each do |item|
        invoice.items.create!({
          product_id: item.product_id,
          quantity: item.quantity,
          unit_price: item.unit_price,
        })
      end

      @order.update(status: :confirmed)
    end

    if @notify
      OrderMailer.preparation_details(@order).deliver_async
      OrderMailer.confirmation(@order).deliver_async
      OrderMailer.available_invoice(@order).deliver_async
    end

  rescue Payment::CaptureError => error
    add_error(:payment_capture_error)
  end
end

It is machine-readable. Does that mean that you can read that easily? Nope… I explain a bit what the code is doing in the other article but it isn’t enough and more importantly, do we want to maintain an up to date documentation for everything?

Use the method Luke

One of the technique I use is to abstract things in methods. As we do with variable, picking relevant name could create a narrative that’s much easier to follow.

For instance, let’s look at that validate block, starting with this:

if payment.amount != @order.sales_quote.amount || payment.currency != @order.sales_quote.currency
  add_error(:payment_amount_mismatch)
end

We don’t want to clutter the reader’s mind with the detail of the conditional. It doesn’t even fit on my screen! We could instead wrap this in a method:

add_error(:payment_amount_mismatch) unless payment_matches_sales_quote?

This doesn’t put all the details in the validate block. And I think it is for the best because when I read that block, I would like to have an overview of what’s validated, not every single implementation details.

We could apply the same for the other conditional and get this:

validate do
  add_error(:payment_token_invalid) unless valid_payment_token?
  add_error(:payment_amount_mismatch) unless payment_amount_matches_sales_quote?
end

To make this happen, we need to define 3 private methods:

private

def payment
  @payment ||= @order.payments.new(token: @payment_token)
end

def valid_payment_token?
  payment.valid?
end

def payment_matches_sales_quote?
  payment.amount == @order.sales_quote.amount &&
  payment.currency == @order.sales_quote.currency
end

After that we end up with more lines in the file but the validate block can now provide a faster understanding to anyone reading the class.

It is possible to go to an higher level of narrative with something like:

validate do
  payment_token_must_be_valid
  payment_amount_must_match_sales_quote
end

You’re the judge of the abstraction level you want to give. The first refactoring uses unless. It is understandable by any Ruby developer. The second version could be understood even by the business person asking for that feature.

Taking a few steps further

If we continue to apply this to the perform block, we could end up with something like:

perform do
  capture_payment!
  create_invoice_and_update_status
  send_notifications

rescue Payment::CaptureError
  add_error(:payment_capture_error)
end

If you’re interested in the create_invoice_and_update_status, you’re free to dig deeper, but if you don’t you have the choice not to bother with the details.

Conclusion

By creating different narratives you can optimize for different targets. Targeting the business forces you to think in the same mindset which has great communication benefits.

You can find the gist of the code here