After writing this post, I discovered an earlier thread on the topic
(http://groups.google.com/group/datamapper/browse_thread/thread/
dc822ad2951a7078/e1142b59af6fb3a1).

While DataMapper is generally excellent, I would like Sam to
reconsider his opposition to using exceptions for save failures.

Right now, model#save returns true or false.  If false is returned,
you must figure out what happened, and you must know each and every
case for why false was returned.

Exceptions are useful to stop execution and to return a meaningful
reason for that - this is exactly what I usually want when a save
fails.

Right now I can stop execution with an "if user.save," but I need to
probe the object for the reason - is it invalid? is there some other
issue?  Any reason in the future for a save failure, such as low level
permissions integrated as a dm module, will not be properly reported,
since my code won't know how to probe for that reason.  If there is an
exception, at least it will say PermissionsError and it will be
handled by the framework with a meaningful error message.

Unless !valid? is the ONLY reason for the save failure, how do I know
what to do with a failure?  Just returning false seems to be a C way
of doing things and not taking advantage of the language.  False
doesn't mean much.

An argument could be made that in some cases, save failure is not an
exceptional case that would invalidate any code that follows it, but
most of the time it requires a rollback and for the initiation of
exception handling.  99% of the time, you will do an "if
model.save ....  else....".  The only time that this is not true is
when you don't care that much if the object was saved, perhaps when
saving to a cache.  (Even that case, I'd prefer explicitly discarding
and logging the specific reason for failure, which is not currently
possible with DM).

AR handles this by having a save! alternative that throws
RecordInvalid.  DataMapper does not appear to offer such a method.
(Instead save! appears to override validation in DM??? - which is
perhaps a cruel joke to those coming from AR - what was wrong with save
(false) or save(nil)?).

Down to the trenches, I'm having trouble coming up with a clean
pattern for handling validation errors the DM way when the save is
handled within a nested method.

For instance:

class Users #controller
  def activate
    ...
    user.activate!
  end
end

class User #model
  def activate!
      if can_activate?
        self.activated = true
        self.save # returns false if invalid
     else
        raise ActivationFailed(self)
     end
  end
end

If activate returns false, it's not necessarily safe to assume it was
because the object is invalid, so I would need to either do this:

def activate
  if user.valid?
    unless user.activate!
      # we don't know why it didn't activate - maybe it was valid when
we passed it into activate,
      # but activation made it invalid (e.g. payment information is
checked on validated on activation)
      raise exception ???
    end
  else
    render(:same_page)
  end
end

OR

def activate
  unless user.activate!
    if user.valid?
      raise exception??? # we don't know why it didn't save in this
case...
    else
      render(:same_page)
    end
  end
end

I'd much rather do this:

def activate
  user.activate!  # activate throws an exception always if activation
does not succeed
  # We handle exception cases here that we don't want handled by the
framework
  # If some other exception that we want handled at the framework
level occurs (e.g. PermissionsError)
  # we can let that bubble up to exceptions.rb - it's clean and pretty
- so much better than the above
rescue RecordInvalid => e
  render(:same_page)
end

which I could accomplish like this:

class User #model
  def activate!
      self.activated = true
      unless save
        if valid?
          # does save returning false ALWAYS mean the object is
invalid?
          # what other cases do I need to look for in the case of save
failure?
          # will more failure cases be added in the future?
          raise UnknownSaveFailure
        else
          raise CustomRecordInvalid(self)
        end
  end
end

I much prefer DM to handle the exception raising, since that is really
where the exception occurs (the object doesn't save because it is
invalid), and I know exactly why it fails.

Maybe someone can suggest a better pattern or alternative?  I can not
find any good examples of something like this the DM way.

I am not religious because I believe that false or unconsidered
beliefs are the source of most evil in the world.  I would ask Sam
that you reconsider your "religion" in this case.

Alternatively, perhaps you can provide me with some recommendation
based on your principles on how to a) know what happened in a
guaranteed future-proof way when save fails and b) return meaningful
failure information from a nested method that calls save.

You suggest a plugin to handle this, but that would not necessarily
solve the problem, since I still don't know if false means invalid.

Thanks, and thanks for a lovely ORM.  But please start using
meaningful exceptions :)





--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"DataMapper" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/datamapper?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to