Very interesting point. So in the end you are recommending to raise only in order to prevent further damage right?
In my current project there are quite a lot .saves and failing saves can lead there in a lot of cases to unpredictable/unwanted behaviour. (And also did in the recent time...) But yeah at the end of the day they all need to be handled properly. My point was just that "failing fast and hard" would be a better default behaviour simplifying the development... So I guess it's quite a good thing that DM supports both ways. Philip 2010/7/20 Jordan Ritter <[email protected]> > 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]<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.
