HiveBrain v1.2.0
Get Started
← Back to all entries
debugrubyrailsModerate

Handling nil: Trying to avoid #try

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
handlingtryingavoidniltry

Problem

Yesterday, I introduced a bug into our codebase by calling #titleize on a string that could possibly come in as nil:

def route
  medication.route_name.titleize
end


This threw an error on production and a coworker fixed this by adding try:

def route
  medication.route_name.try(:titleize)
end


I'm not totally happy with this solution, after being really influenced by Avdi's article on #try, and I've been trying to come up with something I like better. Here are two possibilities.

Firstly, in Avdi's talk "Confident Code", he talks about asking for the data type you need: if you need a string, ask for a string. So I thought about this:

def route
  medication.route_name.to_s.titleize
end


This way, nil will be converted to empty string, and #titleize will be called, which will not error. The obvious issue with this is that you're calling #to_s on something which is only ever an instance of String or NilClass.

The only other approach I could think of was this:

def route
  medication.route_name.present? medication.route_name.titleize : ''
end


The issue here is the if/else parading around as prettier code through the use of a ternary operator, and the repetition of medication.route_name. This could be minified by delegating route_name to medication, or some other work around. But there's still the if/else conditional. And that's more code.

The bigger question here is how to handle nil values. This method is a pretty typical example of how this comes up. How would you handle it?

Solution

I introduced a bug into our codebase by calling #titleize on a string that could possibly come in as nil

When a problem is so pervasive, it's usually a sign that there is a conceptual problem with the language itself, some call it void safety, also dubbed "the billion-dollar mistake". Some languages, specially the functional ones, use an Option type. Let's see the options in Ruby:

-
medication.route_name.try(:titleize). This is probably the most idiomatic approach in Rails. You say you're not totally happy, yes, it's ugly that a method name ended up being a symbol. Note, however, that if you want an empty string as fallback, you should write medication.route_name.try(:titleize) || "".

-
Very similar to try, I prefer the proxy approach of andand/maybe: medication.route_name.maybe.titleize || "".

-
medication.route_name.to_s.titleize. IMO, that's dubious. You are calling to_s to a nil object. Yes, it happens to return "", but it could as well return "nil" (see for example Python: str(None) #=> 'None'). Too implicit.

-
medication.route_name.present? medication.route_name.titleize : ''. You are protecting yourself against nil objects, not empty strings, so there is no need to use present?: medication.route_name ? medication.route_name.titleize : ''. As you say, this is simple but verbose. Also, imagine you have a chain of nullable values, it becomes a real mess. That's why andand and similar libraries were created.

By the way, I've wrote about this subject at RubyIdioms.

Context

StackExchange Code Review Q#28610, answer score: 10

Revisions (0)

No revisions yet.