I am thinking of the best practices of flow control. Which way should I go?
1) Don't check anything and let the program fail (cleaner code, natural error messages):
def self.fetch(feed_id)
feed = Feed.find(feed_id)
feed.fetch
end
2) Fail silently by returning nil (however, "Clean Code" says, that you should never return null):
def self.fetch(feed_id)
return unless feed_id
feed = Feed.find(feed_id)
return unless feed
feed.fetch
end
3) Throw exceptions (because it's exceptional not to find a feed by id):
def self.fetch(feed_id)
raise ArgumentError.new unless feed_id
feed = Feed.find(feed_id)
raise ArgumentError.new unless feed
feed.fetch
end
To put it in other words: should I be using guard conditions actively, or is it better to rely on Ruby / Rails methods and let them throw an exception, if something wrong happens?
if false or if nil won't execute the corresponding condition, because false and nil are considered as falsy values. In other words, if you cast nil or false as a boolean, it will return false . Every other kind of value is considered truthy in Ruby.
Everything in Ruby has a return value! You may notice that the puts and print methods, when run in IRB, print values on the screen and then display a line like this: => nil . This is because puts and print may print the value you want, but instead of returning that value, they return nil .
By default methods in Ruby are empty, and an empty method returns, by default, nil.
Ruby also provides a separate class for an exception that is known as an Exception class which contains different types of methods. The code in which an exception is raised, is enclosed between the begin/end block, so you can use a rescue clause to handle this type of exception.
1) Don't check anything and let the program fail (cleaner code, natural error messages):
It's ok to "let the program fail" with known, documented exceptions, but getting an unpleasant NoMethodError
because you tried to use a nil
object is just careless. In your particular example, ActiveRecord#find
raises a documented ActiveRecord::RecordNotFound
exception, so IMO this is the way to go:
def self.fetch(feed_id)
Feed.find(feed_id).fetch
end
2) Fail silently by returning nil (however, "Clean Code" says, that you should never return null):
That's fine as a general advice, but Ruby is crammed with methods that return nil
; and that's ok (again, as long as it's documented), it just means "Nothing" (and allows the very compact pattern something_that_can_be_nil || another_value
). In this case I'd write it concisely using Ick's maybe
:
def self.fetch(feed_id)
Feed.find_by_id(feed_id).maybe.fetch
end
3) Throw exceptions (because it's exceptional not to find a feed by id):
Yes, but then let the method raise the well-known RecordNotFound
exception, not a custom one (unless you want to abstract the fact that you are working with AR, which can be very cumbersome).
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With