Well the point is that saving without raising an error is the dangerous
version. Instead of having to write "handle_error unless o.save" in a real
world app you often just have to write "o.save" (o.save!..) This saves code.
E.g.:
# encapsulate the db stuff in a function
def some_db_function a
# ...
o.save
my_ret_value
end
def some_func
begin
a, b = do_some_stuff
res = some_db_function a
do_other_stuff res, b
rescue => e
# Some smart error handling that maybe restarts the whole operation
# but uses some more safe conditions in the first place
# ...
end
end
# ...
some_func
# ...
Using the boolean variant you end up using Exceptions anyway for a efficent
error handling -- or you use a bunch a boilerplate code to end up at the
right point of the stack to retry or just give up the operation.
Thinking about the whole problem I can't really image a real world app that
would be more efficient to write using the C like error-handling. Image a
lazy person that is to lazy to create any error handling facility. With the
raise-on-error version no problem: To whole program interrupts if the code
is faulty, he changes it, restarts and it works. Using the boolean-version
the lazy person always needs explicit error handling to catch trivial
writing errors (E.g. forgetting to set a required variable that isn't used
anyway...)
Philip
2010/7/20 Paul Barry <[email protected]>
> How about adopting the ActiveRecord convention of save that returns a
> boolean and save! that raises an exception?
>
>
> On Mon, Jul 19, 2010 at 11:46 PM, David Masover <[email protected]>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]<datamapper%[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]<datamapper%[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.
