IME, the meanings of "safe vs. bang" methods and "what class of errors do
exceptions represent" have always been, at their core, a conscious
philosophical difference between DM and AR.
One (much older) school of thought: exceptions are "exceptional behaviour" --
they typically represent "unrecoverable" events. Divide by zero is an example;
failure to save an object to a DB because of validations is not; failure to
save an object to a DB because of a driver failure (e.g. DO Mysql) is. Error
return values (bool, int, nil vs. not, etc) are for communicating recoverable
failures. In DM's specific context, the code pattern of catching recoverable
errors is minimal, well-defined and easy to read. It also provides for the
"shortcircuit on error" code pattern, which in properly defensive code keeps
the linear flow of code limited to success conditions. All sorts of long-term
benefits ensue from being able to match code comments/logic explanation to
actual code implementation and flow.
One (more recent) school of thought: generally associated with "fail fast and
hard" - everything is an exception. Exception meanings are expanded to mean
"error of any kind", and it's up to the developer to figure out what to do with
it, depending on where they rescue/catch it. While it makes for convenient,
DRY catch points (e.g. by controller) for common classes of exceptions,
system/application state tends to be indeterminate because Exceptions jump out
of stack frames and scopes (this is really, really important). Something that
could be recoverable, isn't, unless you rescue/catch immediately. And even
then, depending on the type of Exception, still maybe isn't. Maybe.
Outside of the development process, Production-quality, hardened core systems
are not allowed to fail unless absolutely necessary. Anyone who has ever built
and *evolved* anything at scale within the last 15 years knows the importance
of this. "Failing hard and fast" is a great development ideology but a piss
poor Production-class mantra. And Production-quality, hardened code does not
ever suffer lazy programmers well, ever. Regardless of what school of thought
they come from.
So, I see this thread boiling down to a simple difference in school of thought:
If you ascribe to the former, then the basic pattern "unless
object.save ... end" is easy enough. Also very simple to DRY up with
app-specific CRUD helpers. If an exception occurs then those are typically
unrecoverable anyway, thus those DRY exception catch points are a good thing to
have regardless.
If you ascribe to the latter, you either have to live with the typical
loss of being able to gracefully recover from non-critical failures (and the
question of indeterminate state), or you employ begin/rescue/end blocks (and
the occasional ensure) everywhere you think you could recover gracefully. In
its initial form, that's sorta nasty looking (as you mentioned below). Those
too can be DRY'd up with CRUD helpers, so this is not such a strong argument
against.
It's flat out wrong to assert DM encourages lazy behaviour: it merely advocates
a different school of thought. In neither case does it advocate people being
lazy (outside of an occasional doc fail). But the fact that you have
raise_on_error in DM means you get your choice of either school, so I guess I
don't understand your point.
cheers,
--jordan
PS: over the last few years I've been seen plenty of newcomers talk about
making it easy for programmers to simply not worry about things like contextual
error handling or code defensiveness, and from a RAD perspective they're right:
the focus should be on building the app. Let the tests identify these
problems. But the hard truth is that, 99 times out of 100, the alpha becomes
the beta becomes the shipping product. Facilitating and encouraging
engineering laziness has *never* lead to good outcomes -- you've got to write
those defenses somewhere, either as edge case tests in the specs, or as edge
case tests in the code. All else being roughly equal, it's just a personal
preference WRT where you do it and how that drives your development process
(though IME writing defensive code often repels classes of real-time attacks
that maybe you didn't specifically think of test for, thus the choice actually
can matter). Regardless though, there's *always* a line to cross where making
it easier to not worry about important things *is* facilitating/encouraging
laziness, and guarantees future disasters. I think most of the professionals
on this list would agree.
On Jul 19, 2010, at 8:46 PM, David Masover wrote:
> On Sunday, July 18, 2010 11:59:41 am Martin Gamsjaeger wrote:
>> I think the motivation behind DM not raising an exception on any save
>> failure by default is a very simple and intuitive one.
>>
>> DM cannot assume to know exactly when a failing save can really be
>> considered *exceptional* behavior.
>
> Really?
>
> irb(main):001:0> 1/0
> ZeroDivisionError: divided by 0
> from (irb):1:in `/'
> from (irb):1
> from /home/ruby19/bin/irb:12:in `<main>'
>
> Who is the core library to say that this is an exception? Why not just create
> an "Indefinite" constant and return that? Or take the limit and return
> Infinity?
>
> Because it IS exceptional behavior. If it's not, that's why we have catch --
> and there are things we can do to avoid the exception. For example, in this
> case, I could've simply checked myself if it was 0 and done something else.
>
>> Rather than forcing the user to
>> treat every single save failure as an exception (which simply isn't
>> true), it decided to inform users in a friendly and more quiet way,
>
> I'm sorry, no, it's not "more friendly" to force me to revert to C-style
> code,
> where I have to check the return value of every other function to make sure
> something didn't fail.
>
> Things should fail early and noisily. If it truly wasn't exceptional
> behavior,
> that's OK, I can deal with that. The minor irritation of occasionally having
> to deal with an exception is well worth avoiding SILENTLY IGNORED ERRORS,
> which is what DM's default behavior entails.
>
>> that doesn't involve the overhead of raising an exception either.
>
> This might be a valid reason. I can also see cases like make_resourceful,
> where it's nice to be able to do this:
>
> if current_object.save
> ...
> else
> ...
>
> So yes, it makes library code mildly nicer, at the cost of making application
> code much more difficult to debug. I don't think that's a good trade,
> especially considering that it actually makes things worse, if people were to
> ever actually use raise_on_save_failure. Now, instead of my library code
> looking like this:
>
> begin
> current_object.save
> ...
> rescue whatever
> ...
> end
>
> It'd look like this:
>
> old_rosf = current_object.raise_on_save_failure
> if current_object.save
> save_successful!
> else
> save_failed!
> end
> current_object.raise_on_save_failure = old_rosf
>
> Wait, that's the naive, non-threadsafe way to do it. Guess I have no choice
> but to do it like this:
>
> if current_object.raise_on_save_failure
> begin
> current_object.save
> save_successful!
> rescue whatever
> save_failed!
> end
> elsif current_object.save
> save_successful!
> else
> save_failed!
> end
>
> This is, bar none, my least favorite thing about DataMapper. I think I might
> turn raise_on_save_failure on globally and see what breaks. Short of that,
> I've actually written code like this:
>
> module Base
> def self.included klass
> klass.class_eval do
> include DataMapper::Resource
> include ClassMethods
> end
> end
> ...
>
> # And now, how save should have worked:
> def store
> save || raise "Save failed!"
> end
> def store!
> save
> end
> def store!!
> save!
> end
> end
>
>
> That way, I can safely use plugins which assume the default behavior, but
> (hopefully) my own code will be somewhat safe.
>
> But please, please make it the default, and provide a _separate_method_ for
> people to use when they want true/false returned, preferably something that
> makes it obvious how dangerous it is. Think about it -- the current behavior
> is effectively setting whether saves are exceptional based on the object (or
> class, or application), and not on the context -- but it's the context which
> defines how errors are handled, if, indeed, they're handled at all.
>
> Or does anyone, ever, think something like this is a good idea:
>
> u = User.new
> u.name = 'John Smith'
> u.save
> puts 'Saved successfully! (I hope.)'
>
> ...really? Would you ever just blindly fire off a save and not care whether
> it
> succeeded or not?
>
> Then why does your public API encourage this by making it so easy to do, even
> by accident?
>
> --
> 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.
>
--
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.
