Hi, just stumbled upon a need to raise a NotFound exception when the object doesn''t have a necessary association. So instead of writing `Company.find(current_user.company_id)` or `current_user.company || raise AR::RecordNotFound`, maybe it would be nicer to just say `current_user.company!`. What do you think? -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscribe@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/groups/opt_out.
+1 2013/9/24 Roman <broilerster@gmail.com>> Hi, > just stumbled upon a need to raise a NotFound exception when the object > doesn''t have a necessary association. So instead of writing > `Company.find(current_user.company_id)` or `current_user.company || raise > AR::RecordNotFound`, maybe it would be nicer to just say > `current_user.company!`. What do you think? > > -- > You received this message because you are subscribed to the Google Groups > "Ruby on Rails: Core" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to rubyonrails-core+unsubscribe@googlegroups.com. > To post to this group, send email to rubyonrails-core@googlegroups.com. > Visit this group at http://groups.google.com/group/rubyonrails-core. > For more options, visit https://groups.google.com/groups/opt_out. >-- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscribe@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/groups/opt_out.
I dislike this. I''d rather see something along the lines of `present?`
that
can be tacked on anywhere. Maybe something like this:
company = current_user.company.present!
--
Josh Susser
http://blog.hasmanythrough.com
On Tue, Sep 24, 2013 at 2:58 PM, Roman <broilerster@gmail.com> wrote:
> Hi,
> just stumbled upon a need to raise a NotFound exception when the object
> doesn''t have a necessary association. So instead of writing
> `Company.find(current_user.company_id)` or `current_user.company || raise
> AR::RecordNotFound`, maybe it would be nicer to just say
> `current_user.company!`. What do you think?
>
> --
> You received this message because you are subscribed to the Google Groups
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rubyonrails-core+unsubscribe@googlegroups.com.
> To post to this group, send email to rubyonrails-core@googlegroups.com.
> Visit this group at http://groups.google.com/group/rubyonrails-core.
> For more options, visit https://groups.google.com/groups/opt_out.
>
--
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to rubyonrails-core+unsubscribe@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at http://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/groups/opt_out.
On Tue, Sep 24, 2013 at 11:58 PM, Roman <broilerster@gmail.com> wrote: Hi,> just stumbled upon a need to raise a NotFound exception when the object > doesn''t have a necessary association. So instead of writing > `Company.find(current_user.company_id)` or `current_user.company || raise > AR::RecordNotFound`, maybe it would be nicer to just say > `current_user.company!`. What do you think? >Do you have a real use case to share for that need? -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscribe@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/groups/opt_out.
Not plussing or minusing this, just want to say if we did something like Josh''s preferred syntax then it should be presence!, not present!, to match presence vs. present?. Brian Morearty On Tuesday, September 24, 2013, Josh Susser wrote:> I dislike this. I''d rather see something along the lines of `present?` > that can be tacked on anywhere. Maybe something like this: > > company = current_user.company.present! > > -- > Josh Susser > http://blog.hasmanythrough.com > > > On Tue, Sep 24, 2013 at 2:58 PM, Roman <broilerster@gmail.com<javascript:_e({}, ''cvml'', ''broilerster@gmail.com'');> > > wrote: > >> Hi, >> just stumbled upon a need to raise a NotFound exception when the object >> doesn''t have a necessary association. So instead of writing >> `Company.find(current_user.company_id)` or `current_user.company || raise >> AR::RecordNotFound`, maybe it would be nicer to just say >> `current_user.company!`. What do you think? >> >> -- >> You received this message because you are subscribed to the Google Groups >> "Ruby on Rails: Core" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to rubyonrails-core+unsubscribe@googlegroups.com<javascript:_e({}, ''cvml'', >> ''rubyonrails-core%2Bunsubscribe@googlegroups.com'');>. >> To post to this group, send email to rubyonrails-core@googlegroups.com<javascript:_e({}, ''cvml'', ''rubyonrails-core@googlegroups.com'');> >> . >> Visit this group at http://groups.google.com/group/rubyonrails-core. >> For more options, visit https://groups.google.com/groups/opt_out. >> > > -- > You received this message because you are subscribed to the Google Groups > "Ruby on Rails: Core" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to rubyonrails-core+unsubscribe@googlegroups.com <javascript:_e({}, > ''cvml'', ''rubyonrails-core%2Bunsubscribe@googlegroups.com'');>. > To post to this group, send email to rubyonrails-core@googlegroups.com<javascript:_e({}, ''cvml'', ''rubyonrails-core@googlegroups.com'');> > . > Visit this group at http://groups.google.com/group/rubyonrails-core. > For more options, visit https://groups.google.com/groups/opt_out. >-- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscribe@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/groups/opt_out.
Using exceptions as flow control is an anti-pattern, so I''d prefer to *not *add this. On Tue, Sep 24, 2013 at 6:51 PM, pathé Sene <pathe.sene@gmail.com> wrote:> +1 > > > 2013/9/24 Roman <broilerster@gmail.com> > >> Hi, >> just stumbled upon a need to raise a NotFound exception when the object >> doesn''t have a necessary association. So instead of writing >> `Company.find(current_user.company_id)` or `current_user.company || raise >> AR::RecordNotFound`, maybe it would be nicer to just say >> `current_user.company!`. What do you think? >> >> -- >> You received this message because you are subscribed to the Google Groups >> "Ruby on Rails: Core" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to rubyonrails-core+unsubscribe@googlegroups.com. >> To post to this group, send email to rubyonrails-core@googlegroups.com. >> Visit this group at http://groups.google.com/group/rubyonrails-core. >> For more options, visit https://groups.google.com/groups/opt_out. >> > > -- > You received this message because you are subscribed to the Google Groups > "Ruby on Rails: Core" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to rubyonrails-core+unsubscribe@googlegroups.com. > To post to this group, send email to rubyonrails-core@googlegroups.com. > Visit this group at http://groups.google.com/group/rubyonrails-core. > For more options, visit https://groups.google.com/groups/opt_out. >-- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscribe@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/groups/opt_out.
Welllll... No one said anything about using this for flow control. On Tuesday, September 24, 2013, James Coleman wrote:> Using exceptions as flow control is an anti-pattern, so I''d prefer to *not > *add this. > > > On Tue, Sep 24, 2013 at 6:51 PM, pathé Sene <pathe.sene@gmail.com<javascript:_e({}, ''cvml'', ''pathe.sene@gmail.com'');> > > wrote: > >> +1 >> >> >> 2013/9/24 Roman <broilerster@gmail.com <javascript:_e({}, ''cvml'', >> ''broilerster@gmail.com'');>> >> >>> Hi, >>> just stumbled upon a need to raise a NotFound exception when the object >>> doesn''t have a necessary association. So instead of writing >>> `Company.find(current_user.company_id)` or `current_user.company || raise >>> AR::RecordNotFound`, maybe it would be nicer to just say >>> `current_user.company!`. What do you think? >>> >>> -- >>> You received this message because you are subscribed to the Google >>> Groups "Ruby on Rails: Core" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to rubyonrails-core+unsubscribe@googlegroups.com<javascript:_e({}, ''cvml'', >>> ''rubyonrails-core%2Bunsubscribe@googlegroups.com'');>. >>> To post to this group, send email to rubyonrails-core@googlegroups.com<javascript:_e({}, ''cvml'', ''rubyonrails-core@googlegroups.com'');> >>> . >>> Visit this group at http://groups.google.com/group/rubyonrails-core. >>> For more options, visit https://groups.google.com/groups/opt_out. >>> >> >> -- >> You received this message because you are subscribed to the Google Groups >> "Ruby on Rails: Core" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to rubyonrails-core+unsubscribe@googlegroups.com<javascript:_e({}, ''cvml'', >> ''rubyonrails-core%2Bunsubscribe@googlegroups.com'');>. >> To post to this group, send email to rubyonrails-core@googlegroups.com<javascript:_e({}, ''cvml'', ''rubyonrails-core@googlegroups.com'');> >> . >> Visit this group at http://groups.google.com/group/rubyonrails-core. >> For more options, visit https://groups.google.com/groups/opt_out. >> > > -- > You received this message because you are subscribed to the Google Groups > "Ruby on Rails: Core" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to rubyonrails-core+unsubscribe@googlegroups.com <javascript:_e({}, > ''cvml'', ''rubyonrails-core%2Bunsubscribe@googlegroups.com'');>. > To post to this group, send email to rubyonrails-core@googlegroups.com<javascript:_e({}, ''cvml'', ''rubyonrails-core@googlegroups.com'');> > . > Visit this group at http://groups.google.com/group/rubyonrails-core. > For more options, visit https://groups.google.com/groups/opt_out. >-- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscribe@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/groups/opt_out.
On 25/09/2013, at 3:17 PM, James Coleman <jtc331@gmail.com> wrote:> Using exceptions as flow control is an anti-pattern, so I''d prefer to not add this.Every time anyone does anything with exceptions an answer like this comes up. People confuse "don''t use exceptions as flow control" for "never ever use exceptions omg what''s wrong with you?!". If you *know* your code is going to do something stupid if a customer doesn''t have an associated credit card, and that every customer passed into this code should *always* have an associated credit card, then having code like this makes perfect sense. def charge_customer_money(customer, amount) if customer.credit_card.nil? raise ArgumentError, "Attempted to charge #{customer.inspect} #{amount.inspect} but there was no associated credit card" end … It''s just simple defensive programming, you''re often better off getting an exception that crashes you than maybe charging the wrong user or causing an exception deep inside your payment gateway library which no one can debug. While this code *could* be used for flow control like: def charge_every_customer Customer.needs_billing.each do |customer| begin charge_customer_money(customer, 4.0) rescue ArgumentError => err print "skipping #{customer.id} because they don''t have a credit card" end end end But it could also be used to simply act as a guard condition in case your billing code accidentally loops over Customer.trialling vs Customer.paying. Don''t be scared of assertions or exceptions, or reject their use cases out of hand.
Michael has given one example. Here''s my turn. When having nested routes like `/trees/42/apples`, I''m doing a `Tree.find(42)` and it raises if there''s no such tree. In other case, when the object is associated to current_user, there''s no need to put its id in the routes, but for symmetry I would still like to call something that conveniently raises if the record is not present. On Wednesday, September 25, 2013 5:50:20 AM UTC+3, Xavier Noria wrote:> > On Tue, Sep 24, 2013 at 11:58 PM, Roman <broil...@gmail.com <javascript:>>wrote: > > Hi, >> just stumbled upon a need to raise a NotFound exception when the object >> doesn''t have a necessary association. So instead of writing >> `Company.find(current_user.company_id)` or `current_user.company || raise >> AR::RecordNotFound`, maybe it would be nicer to just say >> `current_user.company!`. What do you think? >> > > Do you have a real use case to share for that need? > >-- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscribe@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/groups/opt_out.
On Wed, Sep 25, 2013 at 10:33 AM, Roman <broilerster@gmail.com> wrote: Michael has given one example. Here''s my turn. When having nested routes> like `/trees/42/apples`, I''m doing a `Tree.find(42)` and it raises if > there''s no such tree. In other case, when the object is associated to > current_user, there''s no need to put its id in the routes, but for symmetry > I would still like to call something that conveniently raises if the record > is not present. >There is something about the associations API that doesn''t quite match with this proposal in my view. The problem I see is that the associations API is only partially a finder API, due to the cached records. Once you take into account the method does not always fire a query, the semantics of a bang variant start to be unclear to me. Bang methods make sense for me for stuff that goes to the database. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscribe@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/groups/opt_out.
Another mismatch is that a bang method would not translate to plural associations. All in all, I am inclined to suggest that your action should just return an ordinary 404 (not even raise an exception). -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscribe@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/groups/opt_out.
I have some similar feeling along your words "associations API is only partially a finder API", but can''t say exactly what is not right. Why caching matters, and in case of caching why can''t we just return cached object or raise? As for singular/plural mismatch, differences in their interfaces are due to their nature, they already contain lots of examples of this divergence. On Wednesday, September 25, 2013 1:10:36 PM UTC+3, Xavier Noria wrote:> > Another mismatch is that a bang method would not translate to plural > associations. > > All in all, I am inclined to suggest that your action should just return > an ordinary 404 (not even raise an exception). > >-- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscribe@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/groups/opt_out.
On Wed, Sep 25, 2013 at 12:37 PM, Roman <broilerster@gmail.com> wrote: I have some similar feeling along your words "associations API is only> partially a finder API", but can''t say exactly what is not right. Why > caching matters, and in case of caching why can''t we just return cached > object or raise? >I have the same feeling, it doesn''t feel right to me for some reason but cannot put the finger on it. Often a cache is something transparent, an optimization that does not alter the contract, you could remove the cache and reason about the behavior as you did before. In such a case reasoning about public behavior taking the cache into account would be suspicious. In the case of singular associations, the existence of the cache belongs to the contract. That''s why you can do current_user.company.name = ''...'' current_user.company.street = ''...'' In this case the cache is not a mere optimization, belongs to the interface. So my gut feeling says there is something weird in the second call current_user.company!.name = ''...'' current_user.company!.street = ''...'' this alternative also looks strange to me current_user.company!.name = ''...'' current_user.company.street = ''...'' I am trying with those examples to see how this fits with the existing API/usage patterns. In a filter it could make more sense maybe @company = current_user.company! but still. As for singular/plural mismatch, differences in their interfaces are due to> their nature, they already contain lots of examples of this divergence. >Yeah, I believe there is something I don''t like about this particular difference, but this argument/remark is probably weaker. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscribe@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/groups/opt_out.