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]. For more options, visit this group at http://groups.google.com/group/datamapper?hl=en.
