How can I refactor this code?
if env["rack.request.form_hash"] && env["rack.request.form_hash"]["authenticity_token"]
env["rack.request.form_hash"]["authenticity_token"]=env["rack.request.form_hash"]["authenticity_token"].gsub("\r\n",'')
end
-
2shouldn't env be treated as read-only? it's like "params", you can modify it, but you shouldn't.tokland– tokland2011-02-17 16:35:37 +00:00Commented Feb 17, 2011 at 16:35
Add a comment
|
5 Answers
env["rack.request.form_hash"]["authenticity_token"] = env["rack.request.form_hash"]["authenticity_token"].gsub("\r\n",'') rescue nil
or with in place editing
env["rack.request.form_hash"]["authenticity_token"].gsub!("\r\n",'') rescue nil
3 Comments
Simone Carletti
Dear downvoter, it would be nice to know the reason of the downvote.
the Tin Man
I don't know who downvoted it, but it might be because of the
rescue nil. That can cloak exceptions in non-obvious ways so I use it very seldom and carefully, but I'd assume you already knew that.Simone Carletti
I normally avoid
rescue nil because it can be abused, but there's nothing wrong with using it when you actually know what you are doing.if you have the andand gem, you can skip the check and go straight to:
env["rack.request.form_hash"]["authenticity_token"].andand.gsub("\r\n",'')
1 Comment
tokland
isn't a "andand" missing between the hash fetches?
The hash indexes seem to be reused everywhere, maybe you can start there.
key1 = "rack.request.form_hash"
key2 = "authenticity_token"
env[key1] && env[key1][key2]
Nothing clever, but significantly shortens the line.
Something like this could work:
env[key1][key2].gsub!('\r\n','') if env.has_key?(key1) && env[key1].has_key?(key2)
1 Comment
corroded
env[key1][key2] would suffice, since it won't be true if env[key1] was nil in the first place, right? just gotta catch that exception
I would recommend:
if (rrf = env["rack.request.form_hash"]) && rrf_at = rrf["authenticity_token"] then rrf_at.gsub!("\r\n",'') end
or similar but shorter:
rrf_at.gsub!("\r\n",'') if (rrf = env["rack.request.form_hash"]) && rrf_at = rrf["authenticity_token"]
It's DRY, concise and does not use rescue "hacks" ;-D
Comments
Rather then using andand or try, I would do:
if env.fetch("rack.request.form_hash", {})["authenticity_token"].to_s.gsub("\r\n",'')
or add to_hash to the inventory of useful NilClass methods (to_a, to_s, to_i, etc):
class NilClass; def to_hash; {} end end
and do:
if env["rack.request.form_hash"].to_hash["authenticity_token"].to_s.gsub("\r\n",'')
