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?
-
\$\begingroup\$ Smiley for you :-D just for 'Trying to avoid #try' \$\endgroup\$CiaPan– CiaPan2016年06月18日 20:35:38 +00:00Commented Jun 18, 2016 at 20:35
2 Answers 2
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 writemedication.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 callingto_s
to anil
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 againstnil
objects, not empty strings, so there is no need to usepresent?
: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 whyandand
and similar libraries were created.
By the way, I've wrote about this subject at RubyIdioms.
Another solid option is to use the Safe Navigation Operator &. like this:
def route
medication.route_name&.titleize
end
-
\$\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– AlexV2019年05月06日 21:33:47 +00:00Commented May 6, 2019 at 21:33
Explore related questions
See similar questions with these tags.