I'm confused. I was under the impression that the only case in which
save would not raise on error on failure is when validation fails (the
other situations I could think would be an SQL error or connection
failure which both raise errors). In this case, it's not the code that's
the problem but rather user input. Right? Unless the argument is that
you should always call resource.valid? before calling resource.save . At
least in the case of validations causing the failure, I'd much rather
not have to handle an exception (and I don't consider an end-user
mistyping something an exceptional situation). Am I missing something?
Jared
Philip Silva wrote:
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]
<mailto:[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] <mailto:[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 <http://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] <mailto:[email protected]>.
To unsubscribe from this group, send email to
[email protected]
<mailto: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]
<mailto:[email protected]>.
To unsubscribe from this group, send email to
[email protected]
<mailto: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.
--
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.
