Bang-associations

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?

+1

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!

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?

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

Using exceptions as flow control is an anti-pattern, so I’d prefer to not add this.

Welllll… No one said anything about using this for flow control.

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.

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.

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).

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.

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.