# Testing a simple guard clause in isolation

I’m using decorator objects in a Rails app so that I don’t have to do nil? checks in views. Everything is working nicely and it’s easy to test, but I’m suspicious that my test:code ratio is a little high. Am I being too verbose? Is there a better way this could be done?

Test suite:

# spec/decorators/client_decorator_spec.rb
require_relative "../../app/decorators/client_decorator"

describe ClientDecorator do
describe ".position" do
context "with a recorded client position" do
it "returns the name of the position" do
occupation = double("Occupation", name: "CEO")
client = double("Client", occupation: occupation)
expect(ClientDecorator.new(client).position).to eq "CEO"
end
end

context "with no recorded position" do
it "returns an empty string" do
client = double("Client", occupation: nil)
expect(ClientDecorator.new(client).position).to eq ""
end
end
end
end


Production code:

# app/decorators/client_decorator.rb
require "simple_decorator"

class ClientDecorator < SimpleDecorator
def position
return "" unless occupation
occupation.name
end
end


## 2 Answers

Yeah, the code/test ratio is probably a little high - especially if you have to do this for many attributes.

May I direct your attention to the #try method? Seems to me you that you might be able to skip a lot of code (and tests) by doing things like @client.try(:occupation).try(:name).

#try simply returns nil if the receiver doesn't respond to the whatever message you're sending. And since #try is also implemented on the nil object, you can chain "tries" together.

In you views, writing something like <%= @client.try(:occupation).try(:name) %> will just print nothing if the try-chain ends up returning nil - same as your current setup. And if you want a different fallback, you can do this:

<%= @client.try(:occupation).try(:name) || "N/A" %>


In other words, having an entire decorator seems a bit much just to absorb nils. But granted, the view ends up having to dig a bit further into the models.

You can of course also keep the decorator (at least so you current code doesn't break elsewhere), but simply rewrite it using #try to something like

# app/decorators/client_decorator.rb
require "simple_decorator"

class ClientDecorator < SimpleDecorator
def position
occupation.try(:name) || ""
end
end


which I honestly don't know if I'd bother to test very rigorously (lest it breaks horribly).

• Thanks, that’s exactly what I needed. I’ll go with the latter approach; it’s nice to keep try out of the view. I don’t think it needs testing either. – Jezen Thomas Feb 4 '15 at 11:53
• @JezenThomas #try has its uses in views too, though. It's very useful if you need localization for fallback values, for instance. I'd rather handle that in a view, e.g. <%= model.try(:foo) || t(".no_value") %> – Flambino Feb 4 '15 at 12:24
• I think you've got the right idea that "having an entire decorator seems a bit much just to absorb nils", but maybe the best suggestion is to create a base decorator that absorbs nil by default for all methods unless overridden? Then that could be tested once and wouldn't need to be tested for every decorator. – Lee Feb 13 '15 at 17:37
• @Lee It's tempting, but also somewhat blunt as it could camouflage actual errors. It could work of course, but personally I usually prefer to explicitly handle nils where they may be - and seeing things fail if there's a nil where it shouldn't be. – Flambino Feb 13 '15 at 18:19

I dont believe that you are that verbose. The specs are covering the method implementation properly. Something that I would consider improving in your specs ir the repetitive parts of the code. You could DRY your spec with an approach like this:

describe ClientDecorator do
describe "#position" do
subject { decorator.position }
let(:decorator) { ClientDecorator.new(client) }
let(:client) { double "Client", occupation: occupation }

context "with a recorded client position" do
let(:occupation) { double "Occupation", name: "CEO" }
it "returns the name of the position" do
expect(subject).to eq "CEO"
end
end

context "with no recorded position" do
let(:occupation) { nil }
it "returns an empty string" do
expect(subject).to be_blank
end
end
end
end


Also take a look here about how to describe your methods (e.g. # for instance methods and . for class methods). Don't think of it of something as a rule, but rather than a best practise.