8
\$\begingroup\$

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?

\$\endgroup\$
  • \$\begingroup\$ Smiley for you :-D just for 'Trying to avoid #try' \$\endgroup\$ – CiaPan Jun 18 '16 at 20:35
10
\$\begingroup\$

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.

\$\endgroup\$
0
\$\begingroup\$

Another solid option is to use the Safe Navigation Operator &. like this:

def route
  medication.route_name&.titleize
end
\$\endgroup\$
  • \$\begingroup\$ Your linked reference says, that this is available with Ruby 2.3 which was released at the end of 2015, two years after the original question. I would invite you to add a note on this in your answer. \$\endgroup\$ – AlexV May 6 '19 at 21:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.