(Mis)Adventures in Ruby
Think fast! Why is this code wrong?
class User < ActiveRecord::Base
before_save :update_precheck
def update_precheck
check_email_changed
end
# if the user changed their email then make sure we ask them to verify email again
def check_email_changed
if self.email_changed?
self.is_verified = false
end
end
end
If you said “the model will never save any changes if you edit your email address because update_precheck will return false, which cancels the ActiveRecord save” – well congrats! You’re an awesome and experienced Ruby programmer!
For the rest of us, this sucks and is completely unintended behavior because of Ruby’s “I’m so cool I’m going to take the last statement in your function and make it an implicit return value” stunt. There’s a few things at play here: unfortunately this code will work some of the time, because before_save callbacks work if you return nothing OR true, and is only cancelled if you return false. So the lack of strictness in this regard actually hides how the function operates. Just for reference, here is the correct code:
class User < ActiveRecord::Base
before_save :update_precheck
def update_precheck
check_email_changed
end
# if the user changed their email then make sure we ask them to verify email again
def check_email_changed
if self.email_changed?
self.is_verified = false
end
# make sure we dont accidentally cancel saving if this is the last function called by update_precheck
return true
end
end
I try to avoid writing code that accidentally works, but unfortunately Ruby sometimes makes it too easy, apparently.
Edit:
This features reminds me of a good line by one of my heroes, Raymond Chen: Cleaner, More Elegant, and Wrong
