one liner with rescue is good style ?

59 views
Skip to first unread message

wbsu...@gmail.com

unread,
Nov 5, 2015, 3:47:52 PM11/5/15
to Ruby on Rails: Talk

I was reading some ruby one liners. The rescue style seems to make it easy to write short pieces of code, but it feels like I am forcing an error and then just ignoring it in some cases. I guess I am an older programmer and was never encouraged to write this sort of code, but do most people feel like this is good style ?

def receipt_available
   quote_responses.count > 0 && rqstate.eql?("submitted") rescue false
end

obviously if  code had alot of possible errors to recover from you wouldn't do that

Eric Lavigne

unread,
Nov 5, 2015, 4:23:35 PM11/5/15
to rubyonra...@googlegroups.com
This is terrible style and is strictly forbidden in our codebase.

In the example below, probably the programmer thought that an exception always meant that quote_responses was nil instead of an array, and that nil should be treated as an empty array. That works fine... until another source of errors comes along. What if the interface for arrays changed such that count was no longer a valid method, so that this function always returned false? Instead of an obvious exception and a quick fix, the code would just return the wrong answer all the time. (We actually had this sort of problem when switching Rails versions.) Or maybe the name of the rqstate variable changed, and the "undefined variable" exception resulted in this code returning false - same problem.

Something like this is much better. Handle the particular situation that is causing exceptions, so that real problems will remain visible.

def receipt_available
   (quote_responses || []).count > 0 && rqstate.eql?("submitted")
end

Save exception handling for cases where it is really needed. If there's no way to avoid the exception, and you have a way to recover from the exception, then catch only that very specific type of exception. You can also have a high-level exception handler for the entire application that takes care of reporting uncaught exceptions so that they can be fixed.


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-ta...@googlegroups.com.
To post to this group, send email to rubyonra...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/d9c52162-6942-4f29-a2f1-2287af28ae8b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.



--
------------------------
Eric Lavigne
Director of Software Development
MCNA Systems
elav...@mcna.net
Mobile: (352) 871-7829
------------------------
CONFIDENTIALITY NOTICE: This electronic message transmission from MCNA Dental is intended only for the individual or entity to which it is addressed. This e-mail may contain information that is privileged, confidential and exempt from disclosure under applicable law. If you are not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication and attachments is strictly prohibited by the standards set forth in the HITECH Act of 2009. If you received this e-mail by accident, please notify the sender immediately and destroy this e-mail and all copies.  Thank you.
-------------------------
200 W Cypress Creek Rd
Fort Lauderdale, FL

wbsu...@gmail.com

unread,
Nov 5, 2015, 6:01:11 PM11/5/15
to Ruby on Rails: Talk

rqstate is as follows, if there is not a quote request or the status field is null then we could assume
that the request was never submitted to the backend api. This was actually the first time I tried this approach but I am not sure I will keep it this way

def rqstate
   self.quote_request.status rescue "unsubmitted"
end


I got the idea from other uses and #21 here:
http://www.rubyinside.com/21-ruby-tricks-902.html

 I get the point that you are suppressing possible errors, however it seems you could avoid using rescue in many places where it is used.

What if the language made it so you could use that type of syntax and when an error occurred you could handle it someplace else, in effect defining some action for the rescue or is that possible ?

I first took notice of this:

http://stackoverflow.com/questions/800122/best-way-to-convert-strings-to-symbols-in-hash
myhash.keys.each do |key|
  myhash[(key.to_sym rescue key) || key] = myhash.delete(key)
end

I don't quite get why it is not just

myhash[key.to_sym rescue key] = myhash.delete(key)


wbsu...@gmail.com

unread,
Nov 5, 2015, 6:03:35 PM11/5/15
to Ruby on Rails: Talk
I know you can define actions as a block inside of rescue but that wasn't what I meant

wbsu...@gmail.com

unread,
Nov 5, 2015, 6:17:53 PM11/5/15
to Ruby on Rails: Talk

instead of this:


def rqstate
   self.quote_request.status rescue "unsubmitted"
end

I'm going for this, though maybe there is a good one liner I overlooked ?

def rqstate
    ret_res = "unsubmitted"
    ret_res = quote_request.status || ret_res if quote_request
    ret_res
  end

Colin Law

unread,
Nov 6, 2015, 4:04:05 AM11/6/15
to Ruby on Rails: Talk
How about
def rqstate
(quote_request && quote_request.status) ? quote_request.status :
"unsubmitted"
end

Not only a one liner but I think easier to understand. Assuming I
have got it right :)

Colin

Marco Antonio Almeida

unread,
Nov 6, 2015, 4:32:21 AM11/6/15
to rubyonra...@googlegroups.com
On Fri, Nov 6, 2015 at 10:03 AM Colin Law <cla...@gmail.com> wrote:
On 5 November 2015 at 23:17, wbsu...@yahoo.com <wbsu...@gmail.com> wrote:
>
> instead of this:
>
> def rqstate
>    self.quote_request.status rescue "unsubmitted"
> end
>
> I'm going for this, though maybe there is a good one liner I overlooked ?
>
> def rqstate
>     ret_res = "unsubmitted"
>     ret_res = quote_request.status || ret_res if quote_request
>     ret_res
>   end

How about
def rqstate
  (quote_request && quote_request.status)  ?  quote_request.status  :
"unsubmitted"
end

If you're using Rails, another approach is to use try (http://apidock.com/rails/Object/try).

def rqstate
  quote_request.try(:status) || "unsubmitted"
end
 
#try is very nice in this case, but avoid overdoing it. I have faced many codebases with tons of try methods chained and then you lose code readability.


Not only a one liner but I think easier to understand.  Assuming I
have got it right :)

Colin

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-ta...@googlegroups.com.
To post to this group, send email to rubyonra...@googlegroups.com.

Colin Law

unread,
Nov 6, 2015, 5:51:28 AM11/6/15
to Ruby on Rails: Talk
On 6 November 2015 at 09:31, Marco Antonio Almeida
<marco...@gmail.com> wrote:
> On Fri, Nov 6, 2015 at 10:03 AM Colin Law <cla...@gmail.com> wrote:
>> ...
>> How about
>> def rqstate
>> (quote_request && quote_request.status) ? quote_request.status :
>> "unsubmitted"
>> end
>
>
> If you're using Rails, another approach is to use try
> (http://apidock.com/rails/Object/try).
>
> def rqstate
> quote_request.try(:status) || "unsubmitted"
> end
>
> #try is very nice in this case, but avoid overdoing it. I have faced many
> codebases with tons of try methods chained and then you lose code
> readability.

Personally I don't like try. I don't find it easy to intuitively
understand what it is doing, I always seem to have to think about the
code to work out what it is doing. Perhaps that is just my old brain
getting past it though. Of course the ? : syntax can easily become
opaque in more complex cases also.

Colin

>
>>
>> Not only a one liner but I think easier to understand. Assuming I
>> have got it right :)
>>
>> Colin
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Ruby on Rails: Talk" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to rubyonrails-ta...@googlegroups.com.
>> To post to this group, send email to rubyonra...@googlegroups.com.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/rubyonrails-talk/CAL%3D0gLtLLog2%2B3B0EV0gy4jwQLCt5FgUzBKZBRLRmmh95JEJQg%40mail.gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>
> --
> You received this message because you are subscribed to the Google Groups
> "Ruby on Rails: Talk" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rubyonrails-ta...@googlegroups.com.
> To post to this group, send email to rubyonra...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/rubyonrails-talk/CACMkcE41cpqhNK-W9Y9XP2AummBfabOTQguTEtR2WmopR%2B3M%3Dg%40mail.gmail.com.

wbsu...@gmail.com

unread,
Nov 6, 2015, 10:46:27 AM11/6/15
to Ruby on Rails: Talk

 I have been doing alot of contract work lately. I feel like I want to play with some of these one liners as at interviews I sometimes get asked such things but also I want to be flexible to the coding standards of any company I may work at.

Frederick Cheung

unread,
Nov 6, 2015, 11:50:26 AM11/6/15
to Ruby on Rails: Talk

On Friday, November 6, 2015 at 9:32:21 AM UTC, Marco Antonio Almeida wrote:
If you're using Rails, another approach is to use try (http://apidock.com/rails/Object/try).

def rqstate
  quote_request.try(:status) || "unsubmitted"
end
 
#try is very nice in this case, but avoid overdoing it. I have faced many codebases with tons of try methods chained and then you lose code readability.


A gotcha is that if quote_request is non nil, but doesn't actually have a method called status then try will ignore the NoMethodError and just return nil ( try! on the other hand will raise in this case). Prior to rails 4, try behaved like try!

Fred 
Reply all
Reply to author
Forward
0 new messages