Designing an error case

The other day I stumbled upon a seven year old blog post by @dan_menges called Misunderstanding the Law of Demeter. It’s a great post, and it includes a nice code sample for the classic “paperboy” example of Demeter violation:

class Wallet
  attr_accessor :cash
end

class Customer
  has_one :wallet
end

class Paperboy
  def collect_money(customer, due_amount)
    if customer.wallet.cash < due_ammount
      raise InsufficientFundsError
    else
      customer.wallet.cash -= due_amount
      @collected_amount += due_amount
    end
  end
end

I love examples like this, and I like Dan’s improved design later in the post (go read that now). The reason I like this example is because it contains more meat, and therefore more context, than the standard “isn’t customer.getWallet().getCash(amount) terrible!” examples we often see (I’m as guilty of that as anyone). And in that extra context lies an interesting design question…

First though, I’m going to translate Dan’s code from Ruby into Java, so that we can see past some of the syntactic sugar to what’s really happening:

class Wallet {
  public int cash;
}

class Customer {
  public Wallet wallet;
}

class Paperboy {
  private int collected_amount;

  public void collect_money(Customer customer, int due_amount) {
    if (customer.wallet.cash < due_amount)
      throw new InsufficientFundsError();
    customer.wallet.cash -= due_amount;
    collected_amount += due_amount;
  }
}

(This version isn’t identical to the Ruby, which has getter and setter methods for the cash field in the Wallet, but it’s close enough for our purposes.)

Anyway, back to the point. What interests me most about Dan’s code is the InsufficientFundsException. As soon as I saw that exception being thrown, I stopped to think about this code for a good few minutes. And so I have a question for you:

Would you throw that exception there?
If you would, why?
And if you wouldn’t, why not?

Feel free to answer in the comments here, or write your own post and link to it from the comments. I’ll post my thoughts in a few days.

Advertisements

12 thoughts on “Designing an error case

  1. Is “insufficient funds” a truly exceptional circumstance? I’d say no.

    Exceptional would be “customer object is null” or “collected_amount + due_amount > Integer.maxInt”.

  2. I agree with matthewkflint. Exceptions are heavy, blunt instruments and should be used sparingly. The overhead of handling exceptions tends to obscure calling code. I would probably have collect_money() return some kind of status, perhaps just an enum of success or failure, but maybe including more information, depending on what kind of error handling is desired and feasible. In the case of the paperboy, I’d speculate it’s enough to notify the publication of the non-paying customer.

  3. I’d throw the exception in Wallet.withdraw, but I’d also have Wallet.sufficient_funds? I’d also probably have Customer.pay return 0 on insufficient_funds.

  4. I’m leaning towards the exception being the right thing in this context. The collect_money method is a command (in this case more like a demand!) which changes state and CQS tells us that it therefore shouldn’t return data. We could return an error code and the caller might then be able to assume that state had been unchanged but throwing an exception seems a bit stronger / more explicit about the command failing. It also gives the caller the choice of whether to handle it or not rather than forcing the error case to be handled.

  5. I am leaning towards throwing an exception being correct in this context. The collect_money method is a command that changes state and CQS might then tell us that it should not also be returning data. We could return some kind of success / fail status, and the caller might then infer that no state had changed, but throwing an exception seems a bit stronger / more explicit about the command failing. It also gives the caller the choice of if it wants to handle the error or not, rather than forcing it to handle the success / fail status.

  6. Is it exceptional that the customer doesn’t have enough money? I’d argue it isn’t so I would not throw the exception. Quite often a paperboy can ask a customer for money and they say ‘oh you’ll have to come back next week’.

    Could this been seen as separating query from modifier and therefore have two methods on paperboy about the customer, one asking if they have enough money and one deducting the money if they have? (with a third setting a reminder to ask again later)

    • As always it depends on the context, but it might be preferable to keep the paperboy api small and simple. I’ve been playing with Erlang recently and there is a lot to recommend in its “code for the happy path, let the rest crash” philosophy :)

  7. I would not handle what appears (from the context given) to be an alternate flow of the case of use, as an exception. I would throw only things that my application cannot reasonably expect to happen in normal execution. To me, an exception means something truly exceptional or more usually bad, has happened.

  8. My two cents.

    I suppose the example depends on the importance of collecting the customer’s money – that a non-payment would surely be the end of the world.

    Perhaps it would be an acceptable assumpton that from time to time customers will be unable to pay their billson time, they’re human and so surely we wouldn’t add to the collected_amount but would rather to a debt_amount for collection on another date/by another service.

  9. I’d say the contract between Paperboy and Customer is violated, so throwing an exception is okay. The caller of the method can then handle that e.g. by cancelling a subscription.

  10. It’s probably not exceptional that a customer has no money to hand when the paperboy happens to call.

    So, thinking about https://silkandspinach.net/2014/10/19/eliminate-many-conditionals-with-this-one-weird-trick/ I’m tempted to suggest passing in a strategy to the paper boy which would be followed if a customer can’t pay.

    For example, you may allow a customer to accrue debt for a week or two and then stop delivering papers until they go into the shop to pay up.

    If the paperboy is given a callback/delegate that they can pass the customer to then you don’t need to throw the exception, the behaviour of the paper boy can be changed in future or for particular customers, and the paperboy doesn’t have to know too much about what will happen.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s