Changing the default behaviour of save()
Locked
123
123
 
|
Hi,
This discussion has come up a few times, but I think we should revisit it for Grails 2.0. As we all know, save() by default currently returns null if validation failed (or throws an exception if database constraints were violated). The main reason this has never been changed is that doing so would immediately break existing applications unless a configuration option were set. I think that a new major version gives us an opportunity to make the change in the context of other significant changes, especially as users who wanted the existing behaviour could set the default failOnError option to false. That allows us to discuss the default behaviour of save() on each option's merits. I for one favour the approach of least surprise for new users. I have myself often been caught out by saves that quietly failed validation when I wasn't expecting it. The time spent tracking down the underlying issues would have vanished to zero if the saves had simply thrown exceptions. It would be easy for more experienced hands to switch the default and always check the return value or use failOnError: true. To me, the key of the framework is to aid developer productivity, hence why I think there's a strong argument for changing the default behaviour. What do others think? Peter --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
Re: Changing the default behaviour of save()
|
I am in favor of changing the default of failOnError to true, for the exact reasons you cite.
Personally I feel that this is wrong theoretically, but in practice makes sense based off the user feedback over the years. On 01/10/2010, at 7:24 PM, Peter Ledbrook <[hidden email]> wrote: > Hi, > > This discussion has come up a few times, but I think we should revisit > it for Grails 2.0. As we all know, save() by default currently returns > null if validation failed (or throws an exception if database > constraints were violated). The main reason this has never been > changed is that doing so would immediately break existing applications > unless a configuration option were set. I think that a new major > version gives us an opportunity to make the change in the context of > other significant changes, especially as users who wanted the existing > behaviour could set the default failOnError option to false. > > That allows us to discuss the default behaviour of save() on each > option's merits. I for one favour the approach of least surprise for > new users. I have myself often been caught out by saves that quietly > failed validation when I wasn't expecting it. The time spent tracking > down the underlying issues would have vanished to zero if the saves > had simply thrown exceptions. It would be easy for more experienced > hands to switch the default and always check the return value or use > failOnError: true. To me, the key of the framework is to aid developer > productivity, hence why I think there's a strong argument for changing > the default behaviour. > > What do others think? > > Peter > > --------------------------------------------------------------------- > To unsubscribe from this list, please visit: > > http://xircles.codehaus.org/manage_email > > --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
Re: Changing the default behaviour of save()
Administrator
|
+1
On Fri, Oct 1, 2010 at 11:32 AM, Luke Daley <[hidden email]> wrote: > I am in favor of changing the default of failOnError to true, for the exact reasons you cite. > > Personally I feel that this is wrong theoretically, but in practice makes sense based off the user feedback over the years. > > On 01/10/2010, at 7:24 PM, Peter Ledbrook <[hidden email]> wrote: > >> Hi, >> >> This discussion has come up a few times, but I think we should revisit >> it for Grails 2.0. As we all know, save() by default currently returns >> null if validation failed (or throws an exception if database >> constraints were violated). The main reason this has never been >> changed is that doing so would immediately break existing applications >> unless a configuration option were set. I think that a new major >> version gives us an opportunity to make the change in the context of >> other significant changes, especially as users who wanted the existing >> behaviour could set the default failOnError option to false. >> >> That allows us to discuss the default behaviour of save() on each >> option's merits. I for one favour the approach of least surprise for >> new users. I have myself often been caught out by saves that quietly >> failed validation when I wasn't expecting it. The time spent tracking >> down the underlying issues would have vanished to zero if the saves >> had simply thrown exceptions. It would be easy for more experienced >> hands to switch the default and always check the return value or use >> failOnError: true. To me, the key of the framework is to aid developer >> productivity, hence why I think there's a strong argument for changing >> the default behaviour. >> >> What do others think? >> >> Peter >> >> --------------------------------------------------------------------- >> To unsubscribe from this list, please visit: >> >> http://xircles.codehaus.org/manage_email >> >> > > --------------------------------------------------------------------- > To unsubscribe from this list, please visit: > > http://xircles.codehaus.org/manage_email > > > -- Graeme Rocher Grails Project Lead SpringSource - A Division of VMware http://www.springsource.com --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
Re: Changing the default behaviour of save()
|
In reply to this post by ld@ldaley.com
I for one favour the approach of least surprise for
> new users. I have myself often been caught out by saves that quietly > failed validation when I wasn't expecting it. I have difficulty understanding the above. May be you are used to exception based approach and I am used to the other. Programmers coming from backgrounds other than Java are used to null based approach and its natural for them to use save calls in if statements.
Validation is mostly errors in user's data input and used for showing Validation failure messages to users using views. The user corrects the input and sends again. This is a normal process in case of web applications where user inputs data and in my opinion does not qualify to be an Exceptional Condition(which programmers don't expect to occur). Database validation errors should not occur and qualify to be an Exceptional Condition.
Putting the save call in an if clause is easier than using a try with many catch blocks. I wonder what those catch blocks would do in simple operations like the code generated by scaffold generator where the same domain object is used to show errors using views.
It will make sense in long and complex service methods where a complex code is surrounded by try block which can make use of failOnError. But in case of simple controller actions using user input and persisting objects using user's input, the current approach is better and results in more understandable and concise code.
Even if validation is considered an Exceptional Condition a single exception object will not help in deciphering the error cause as those may be multiple validation errors. Grails developers should consider programmer's comfort as a top priority rather than Java idioms as the Grails idea did not extend from any of work done in java. I would rather use Java if my code uses a lot of try catch blocks. Similarly many of other groovy/grails offerings would create hindrance in tracking errors. I love the groovy java mixture and use it for exact reasons as pointed out in this email. Grails itself is an example of this approach. Grails contains java/groovy mixed code and number of try blocks in groovy code are a lot less than the java one.
Khurram Ijaz. On Fri, Oct 1, 2010 at 2:32 PM, Luke Daley <[hidden email]> wrote: I am in favor of changing the default of failOnError to true, for the exact reasons you cite. |
Re: Changing the default behaviour of save()
|
Administrator
|
On Fri, Oct 1, 2010 at 1:13 PM, Khurram Ijaz <[hidden email]> wrote:
> I for one favour the approach of least surprise for >> new users. I have myself often been caught out by saves that quietly >> failed validation when I wasn't expecting it. > I have difficulty understanding the above. May be you are used to exception > based approach and I am used to the other. Programmers coming from > backgrounds other than Java are used to null based approach and its natural > for them to use save calls in if statements. > Validation is mostly errors in user's data input and used for showing > Validation failure messages to users using views. The user corrects the > input and sends again. This is a normal process in case of web applications > where user inputs data and in my opinion does not qualify to be an > Exceptional Condition(which programmers don't expect to occur). Database > validation errors should not occur and qualify to be an Exceptional > Condition. > Putting the save call in an if clause is easier than using a try with many > catch blocks. I wonder what those catch blocks would do in simple operations > like the code generated by scaffold generator where the same domain object > is used to show errors using views. > It will make sense in long and complex service methods where a complex code > is surrounded by try block which can make use of failOnError. But in case of > simple controller actions using user input and persisting objects using > user's input, the current approach is better and results in more > understandable and concise code. > Even if validation is considered an Exceptional Condition a single exception > object will not help in deciphering the error cause as those may be multiple > validation errors. > Grails developers should consider programmer's comfort as a top priority > rather than Java idioms as the Grails idea did not extend from any of work > done in java. I would rather use Java if my code uses a lot of try catch > blocks. Similarly many of other groovy/grails offerings would > create hindrance in tracking errors. I love the groovy java mixture and use > it for exact reasons as pointed out in this email. Grails itself is an > example of this approach. Grails contains java/groovy mixed code and number > of try blocks in groovy code are a lot less than the java one. Some valid points made as a counter argument. We could add another method that throws an exception: obj."save!"() Although the ! notation is more prominent in Ruby since in Groovy you have to quote. Cheers > Khurram Ijaz. > > > On Fri, Oct 1, 2010 at 2:32 PM, Luke Daley <[hidden email]> wrote: >> >> I am in favor of changing the default of failOnError to true, for the >> exact reasons you cite. >> >> Personally I feel that this is wrong theoretically, but in practice makes >> sense based off the user feedback over the years. >> >> On 01/10/2010, at 7:24 PM, Peter Ledbrook <[hidden email]> wrote: >> >> > Hi, >> > >> > This discussion has come up a few times, but I think we should revisit >> > it for Grails 2.0. As we all know, save() by default currently returns >> > null if validation failed (or throws an exception if database >> > constraints were violated). The main reason this has never been >> > changed is that doing so would immediately break existing applications >> > unless a configuration option were set. I think that a new major >> > version gives us an opportunity to make the change in the context of >> > other significant changes, especially as users who wanted the existing >> > behaviour could set the default failOnError option to false. >> > >> > That allows us to discuss the default behaviour of save() on each >> > option's merits. I for one favour the approach of least surprise for >> > new users. I have myself often been caught out by saves that quietly >> > failed validation when I wasn't expecting it. The time spent tracking >> > down the underlying issues would have vanished to zero if the saves >> > had simply thrown exceptions. It would be easy for more experienced >> > hands to switch the default and always check the return value or use >> > failOnError: true. To me, the key of the framework is to aid developer >> > productivity, hence why I think there's a strong argument for changing >> > the default behaviour. >> > >> > What do others think? >> > >> > Peter >> > >> > --------------------------------------------------------------------- >> > To unsubscribe from this list, please visit: >> > >> > http://xircles.codehaus.org/manage_email >> > >> > >> >> --------------------------------------------------------------------- >> To unsubscribe from this list, please visit: >> >> http://xircles.codehaus.org/manage_email >> >> > > -- Graeme Rocher Grails Project Lead SpringSource - A Division of VMware http://www.springsource.com --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
Re: Changing the default behaviour of save()
|
+1 for changing the default behavior.
Colin Harrington [hidden email] On Oct 1, 2010, at 6:49 AM, Graeme Rocher <[hidden email]> wrote: > On Fri, Oct 1, 2010 at 1:13 PM, Khurram Ijaz <[hidden email]> wrote: >> I for one favour the approach of least surprise for >>> new users. I have myself often been caught out by saves that quietly >>> failed validation when I wasn't expecting it. >> I have difficulty understanding the above. May be you are used to exception >> based approach and I am used to the other. Programmers coming from >> backgrounds other than Java are used to null based approach and its natural >> for them to use save calls in if statements. >> Validation is mostly errors in user's data input and used for showing >> Validation failure messages to users using views. The user corrects the >> input and sends again. This is a normal process in case of web applications >> where user inputs data and in my opinion does not qualify to be an >> Exceptional Condition(which programmers don't expect to occur). Database >> validation errors should not occur and qualify to be an Exceptional >> Condition. >> Putting the save call in an if clause is easier than using a try with many >> catch blocks. I wonder what those catch blocks would do in simple operations >> like the code generated by scaffold generator where the same domain object >> is used to show errors using views. >> It will make sense in long and complex service methods where a complex code >> is surrounded by try block which can make use of failOnError. But in case of >> simple controller actions using user input and persisting objects using >> user's input, the current approach is better and results in more >> understandable and concise code. >> Even if validation is considered an Exceptional Condition a single exception >> object will not help in deciphering the error cause as those may be multiple >> validation errors. >> Grails developers should consider programmer's comfort as a top priority >> rather than Java idioms as the Grails idea did not extend from any of work >> done in java. I would rather use Java if my code uses a lot of try catch >> blocks. Similarly many of other groovy/grails offerings would >> create hindrance in tracking errors. I love the groovy java mixture and use >> it for exact reasons as pointed out in this email. Grails itself is an >> example of this approach. Grails contains java/groovy mixed code and number >> of try blocks in groovy code are a lot less than the java one. > > Some valid points made as a counter argument. We could add another > method that throws an exception: > > obj."save!"() > > Although the ! notation is more prominent in Ruby since in Groovy you > have to quote. > > Cheers > >> Khurram Ijaz. >> >> >> On Fri, Oct 1, 2010 at 2:32 PM, Luke Daley <[hidden email]> wrote: >>> >>> I am in favor of changing the default of failOnError to true, for the >>> exact reasons you cite. >>> >>> Personally I feel that this is wrong theoretically, but in practice makes >>> sense based off the user feedback over the years. >>> >>> On 01/10/2010, at 7:24 PM, Peter Ledbrook <[hidden email]> wrote: >>> >>>> Hi, >>>> >>>> This discussion has come up a few times, but I think we should revisit >>>> it for Grails 2.0. As we all know, save() by default currently returns >>>> null if validation failed (or throws an exception if database >>>> constraints were violated). The main reason this has never been >>>> changed is that doing so would immediately break existing applications >>>> unless a configuration option were set. I think that a new major >>>> version gives us an opportunity to make the change in the context of >>>> other significant changes, especially as users who wanted the existing >>>> behaviour could set the default failOnError option to false. >>>> >>>> That allows us to discuss the default behaviour of save() on each >>>> option's merits. I for one favour the approach of least surprise for >>>> new users. I have myself often been caught out by saves that quietly >>>> failed validation when I wasn't expecting it. The time spent tracking >>>> down the underlying issues would have vanished to zero if the saves >>>> had simply thrown exceptions. It would be easy for more experienced >>>> hands to switch the default and always check the return value or use >>>> failOnError: true. To me, the key of the framework is to aid developer >>>> productivity, hence why I think there's a strong argument for changing >>>> the default behaviour. >>>> >>>> What do others think? >>>> >>>> Peter >>>> >>>> --------------------------------------------------------------------- >>>> To unsubscribe from this list, please visit: >>>> >>>> http://xircles.codehaus.org/manage_email >>>> >>>> >>> >>> --------------------------------------------------------------------- >>> To unsubscribe from this list, please visit: >>> >>> http://xircles.codehaus.org/manage_email >>> >>> >> >> > > > > -- > Graeme Rocher > Grails Project Lead > SpringSource - A Division of VMware > http://www.springsource.com > > --------------------------------------------------------------------- > To unsubscribe from this list, please visit: > > http://xircles.codehaus.org/manage_email > > --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
|
after wasting a lot of time on this a few times, I also went to set
failonerror to true, always cheers, andré 2010/10/1 Colin Harrington <[hidden email]>: > +1 for changing the default behavior. > > Colin Harrington > [hidden email] > > On Oct 1, 2010, at 6:49 AM, Graeme Rocher <[hidden email]> wrote: > >> On Fri, Oct 1, 2010 at 1:13 PM, Khurram Ijaz <[hidden email]> wrote: >>> I for one favour the approach of least surprise for >>>> new users. I have myself often been caught out by saves that quietly >>>> failed validation when I wasn't expecting it. >>> I have difficulty understanding the above. May be you are used to exception >>> based approach and I am used to the other. Programmers coming from >>> backgrounds other than Java are used to null based approach and its natural >>> for them to use save calls in if statements. >>> Validation is mostly errors in user's data input and used for showing >>> Validation failure messages to users using views. The user corrects the >>> input and sends again. This is a normal process in case of web applications >>> where user inputs data and in my opinion does not qualify to be an >>> Exceptional Condition(which programmers don't expect to occur). Database >>> validation errors should not occur and qualify to be an Exceptional >>> Condition. >>> Putting the save call in an if clause is easier than using a try with many >>> catch blocks. I wonder what those catch blocks would do in simple operations >>> like the code generated by scaffold generator where the same domain object >>> is used to show errors using views. >>> It will make sense in long and complex service methods where a complex code >>> is surrounded by try block which can make use of failOnError. But in case of >>> simple controller actions using user input and persisting objects using >>> user's input, the current approach is better and results in more >>> understandable and concise code. >>> Even if validation is considered an Exceptional Condition a single exception >>> object will not help in deciphering the error cause as those may be multiple >>> validation errors. >>> Grails developers should consider programmer's comfort as a top priority >>> rather than Java idioms as the Grails idea did not extend from any of work >>> done in java. I would rather use Java if my code uses a lot of try catch >>> blocks. Similarly many of other groovy/grails offerings would >>> create hindrance in tracking errors. I love the groovy java mixture and use >>> it for exact reasons as pointed out in this email. Grails itself is an >>> example of this approach. Grails contains java/groovy mixed code and number >>> of try blocks in groovy code are a lot less than the java one. >> >> Some valid points made as a counter argument. We could add another >> method that throws an exception: >> >> obj."save!"() >> >> Although the ! notation is more prominent in Ruby since in Groovy you >> have to quote. >> >> Cheers >> >>> Khurram Ijaz. >>> >>> >>> On Fri, Oct 1, 2010 at 2:32 PM, Luke Daley <[hidden email]> wrote: >>>> >>>> I am in favor of changing the default of failOnError to true, for the >>>> exact reasons you cite. >>>> >>>> Personally I feel that this is wrong theoretically, but in practice makes >>>> sense based off the user feedback over the years. >>>> >>>> On 01/10/2010, at 7:24 PM, Peter Ledbrook <[hidden email]> wrote: >>>> >>>>> Hi, >>>>> >>>>> This discussion has come up a few times, but I think we should revisit >>>>> it for Grails 2.0. As we all know, save() by default currently returns >>>>> null if validation failed (or throws an exception if database >>>>> constraints were violated). The main reason this has never been >>>>> changed is that doing so would immediately break existing applications >>>>> unless a configuration option were set. I think that a new major >>>>> version gives us an opportunity to make the change in the context of >>>>> other significant changes, especially as users who wanted the existing >>>>> behaviour could set the default failOnError option to false. >>>>> >>>>> That allows us to discuss the default behaviour of save() on each >>>>> option's merits. I for one favour the approach of least surprise for >>>>> new users. I have myself often been caught out by saves that quietly >>>>> failed validation when I wasn't expecting it. The time spent tracking >>>>> down the underlying issues would have vanished to zero if the saves >>>>> had simply thrown exceptions. It would be easy for more experienced >>>>> hands to switch the default and always check the return value or use >>>>> failOnError: true. To me, the key of the framework is to aid developer >>>>> productivity, hence why I think there's a strong argument for changing >>>>> the default behaviour. >>>>> >>>>> What do others think? >>>>> >>>>> Peter >>>>> >>>>> --------------------------------------------------------------------- >>>>> To unsubscribe from this list, please visit: >>>>> >>>>> http://xircles.codehaus.org/manage_email >>>>> >>>>> >>>> >>>> --------------------------------------------------------------------- >>>> To unsubscribe from this list, please visit: >>>> >>>> http://xircles.codehaus.org/manage_email >>>> >>>> >>> >>> >> >> >> >> -- >> Graeme Rocher >> Grails Project Lead >> SpringSource - A Division of VMware >> http://www.springsource.com >> >> --------------------------------------------------------------------- >> To unsubscribe from this list, please visit: >> >> http://xircles.codehaus.org/manage_email >> >> > > --------------------------------------------------------------------- > To unsubscribe from this list, please visit: > > http://xircles.codehaus.org/manage_email > > > --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
Re: Changing the default behaviour of save()
|
+1
Been stung more than once... On 1 Oct 2010, at 15:25, Andre <[hidden email]> wrote: > after wasting a lot of time on this a few times, I also went to set > failonerror to true, always > > cheers, > andré > > 2010/10/1 Colin Harrington <[hidden email]>: >> +1 for changing the default behavior. >> >> Colin Harrington >> [hidden email] >> >> On Oct 1, 2010, at 6:49 AM, Graeme Rocher <[hidden email]> wrote: >> >>> On Fri, Oct 1, 2010 at 1:13 PM, Khurram Ijaz <[hidden email]> wrote: >>>> I for one favour the approach of least surprise for >>>>> new users. I have myself often been caught out by saves that quietly >>>>> failed validation when I wasn't expecting it. >>>> I have difficulty understanding the above. May be you are used to exception >>>> based approach and I am used to the other. Programmers coming from >>>> backgrounds other than Java are used to null based approach and its natural >>>> for them to use save calls in if statements. >>>> Validation is mostly errors in user's data input and used for showing >>>> Validation failure messages to users using views. The user corrects the >>>> input and sends again. This is a normal process in case of web applications >>>> where user inputs data and in my opinion does not qualify to be an >>>> Exceptional Condition(which programmers don't expect to occur). Database >>>> validation errors should not occur and qualify to be an Exceptional >>>> Condition. >>>> Putting the save call in an if clause is easier than using a try with many >>>> catch blocks. I wonder what those catch blocks would do in simple operations >>>> like the code generated by scaffold generator where the same domain object >>>> is used to show errors using views. >>>> It will make sense in long and complex service methods where a complex code >>>> is surrounded by try block which can make use of failOnError. But in case of >>>> simple controller actions using user input and persisting objects using >>>> user's input, the current approach is better and results in more >>>> understandable and concise code. >>>> Even if validation is considered an Exceptional Condition a single exception >>>> object will not help in deciphering the error cause as those may be multiple >>>> validation errors. >>>> Grails developers should consider programmer's comfort as a top priority >>>> rather than Java idioms as the Grails idea did not extend from any of work >>>> done in java. I would rather use Java if my code uses a lot of try catch >>>> blocks. Similarly many of other groovy/grails offerings would >>>> create hindrance in tracking errors. I love the groovy java mixture and use >>>> it for exact reasons as pointed out in this email. Grails itself is an >>>> example of this approach. Grails contains java/groovy mixed code and number >>>> of try blocks in groovy code are a lot less than the java one. >>> >>> Some valid points made as a counter argument. We could add another >>> method that throws an exception: >>> >>> obj."save!"() >>> >>> Although the ! notation is more prominent in Ruby since in Groovy you >>> have to quote. >>> >>> Cheers >>> >>>> Khurram Ijaz. >>>> >>>> >>>> On Fri, Oct 1, 2010 at 2:32 PM, Luke Daley <[hidden email]> wrote: >>>>> >>>>> I am in favor of changing the default of failOnError to true, for the >>>>> exact reasons you cite. >>>>> >>>>> Personally I feel that this is wrong theoretically, but in practice makes >>>>> sense based off the user feedback over the years. >>>>> >>>>> On 01/10/2010, at 7:24 PM, Peter Ledbrook <[hidden email]> wrote: >>>>> >>>>>> Hi, >>>>>> >>>>>> This discussion has come up a few times, but I think we should revisit >>>>>> it for Grails 2.0. As we all know, save() by default currently returns >>>>>> null if validation failed (or throws an exception if database >>>>>> constraints were violated). The main reason this has never been >>>>>> changed is that doing so would immediately break existing applications >>>>>> unless a configuration option were set. I think that a new major >>>>>> version gives us an opportunity to make the change in the context of >>>>>> other significant changes, especially as users who wanted the existing >>>>>> behaviour could set the default failOnError option to false. >>>>>> >>>>>> That allows us to discuss the default behaviour of save() on each >>>>>> option's merits. I for one favour the approach of least surprise for >>>>>> new users. I have myself often been caught out by saves that quietly >>>>>> failed validation when I wasn't expecting it. The time spent tracking >>>>>> down the underlying issues would have vanished to zero if the saves >>>>>> had simply thrown exceptions. It would be easy for more experienced >>>>>> hands to switch the default and always check the return value or use >>>>>> failOnError: true. To me, the key of the framework is to aid developer >>>>>> productivity, hence why I think there's a strong argument for changing >>>>>> the default behaviour. >>>>>> >>>>>> What do others think? >>>>>> >>>>>> Peter >>>>>> >>>>>> --------------------------------------------------------------------- >>>>>> To unsubscribe from this list, please visit: >>>>>> >>>>>> http://xircles.codehaus.org/manage_email >>>>>> >>>>>> >>>>> >>>>> --------------------------------------------------------------------- >>>>> To unsubscribe from this list, please visit: >>>>> >>>>> http://xircles.codehaus.org/manage_email >>>>> >>>>> >>>> >>>> >>> >>> >>> >>> -- >>> Graeme Rocher >>> Grails Project Lead >>> SpringSource - A Division of VMware >>> http://www.springsource.com >>> >>> --------------------------------------------------------------------- >>> To unsubscribe from this list, please visit: >>> >>> http://xircles.codehaus.org/manage_email >>> >>> >> >> --------------------------------------------------------------------- >> To unsubscribe from this list, please visit: >> >> http://xircles.codehaus.org/manage_email >> >> >> > > --------------------------------------------------------------------- > To unsubscribe from this list, please visit: > > http://xircles.codehaus.org/manage_email > > --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
Re: Changing the default behaviour of save()
|
In reply to this post by pledbrook
On Fri, Oct 1, 2010 at 4:24 AM, Peter Ledbrook <[hidden email]> wrote:
> Hi, > > This discussion has come up a few times, but I think we should revisit > it for Grails 2.0. As we all know, save() by default currently returns > null if validation failed (or throws an exception if database > constraints were violated). The main reason this has never been > changed is that doing so would immediately break existing applications > unless a configuration option were set. I think that a new major > version gives us an opportunity to make the change in the context of > other significant changes, especially as users who wanted the existing > behaviour could set the default failOnError option to false. > > That allows us to discuss the default behaviour of save() on each > option's merits. I for one favour the approach of least surprise for > new users. I have myself often been caught out by saves that quietly > failed validation when I wasn't expecting it. The time spent tracking > down the underlying issues would have vanished to zero if the saves > had simply thrown exceptions. It would be easy for more experienced > hands to switch the default and always check the return value or use > failOnError: true. To me, the key of the framework is to aid developer > productivity, hence why I think there's a strong argument for changing > the default behaviour. > > What do others think? > > Peter > Right now it is idiomatic to do something like this... def save = { def widgetInstance = new Widget(params) if (widgetInstance.save(flush: true)) { redirect(action: "show", id: widgetInstance.id) } else { render(view: "create", model: [widgetInstance: widgetInstance]) } } With the change in default behavior, do folks prefer something like this?... def save = { def widgetInstance = new Widget(params) if (widgetInstance.validate() && widgetInstance.save(flush: true)) { redirect(action: "show", id: widgetInstance.id) } else { render(view: "create", model: [widgetInstance: widgetInstance]) } } Or maybe... def save = { def widgetInstance = new Widget(params) try { widgetInstance.save(flush: true) redirect(action: "show", id: widgetInstance.id) } catch (e) { render(view: "create", model: [widgetInstance: widgetInstance]) } } Each of those have their own issues. Maybe there is some other pattern that folks have in mind. I expect I am the only person opposed to the change, and I am fine with that of course. For the record... -1. ;) jb -- Jeff Brown SpringSource http://www.springsource.com/ Autism Strikes 1 in 166 Find The Cause ~ Find The Cure http://www.autismspeaks.org/ --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
Re: Changing the default behaviour of save()
|
|
Jeff,
Good point - error handling would need a try/catch by default or setting failOnError:false. Is this a better convention than the current behavior? In terms of usage, I think failOnError as the default behavior makes a lot of sense from transactional services where the transaction is rolled back upon exception. This is a pattern that I've been using extensively. I think you would use: widgetInstance.save(flush: true, failOnError: false) instead of widgetInstance.validate() && widgetInstance.save(flush: true) to get the current behavior Could we introduce a validateAndSave(flush: true) (or something similar) that acts like the save(failOnError: false)? def save = { def widgetInstance = new Widget(params) if (widgetInstance.validateAndSave(flush: true)) { redirect(action: "show", id: widgetInstance.id) } else { render(view: "create", model: [widgetInstance: widgetInstance]) } } Is it possible that failOnError isn't the best name? It really only adds a ValidationException when the entity doesn't validate. failOnError:true seems to indicate that the save() is expected to persist and will throw an exception if anything goes wrong. Colin Harrington [hidden email] On Fri, Oct 1, 2010 at 11:25 AM, Jeff Brown <[hidden email]> wrote:
|
Re: Changing the default behaviour of save()
|
I'll take the middle ground - I appreciate both possibilities. It would be helpful if Jeff's idea was followed to ensure that save() doesn't call validate() if validate() has already been called and the object is not dirty.
I do think that Graeme's suggestion of having the rubyish object."save!"() would allow a more easily understood way to do either idiom. Because I think that often validation should NOT cause an exception by default since it is not exceptional behavior (i.e. exceptional behavior would be a database level exception). However, in transactional settings, the exception could be helpful - so I'm adding +1 for the save() remaining the same and adding "save!"().
James Coleman
On Fri, Oct 1, 2010 at 1:17 PM, Colin Harrington <[hidden email]> wrote: Jeff, |
Re: Changing the default behaviour of save()
|
In reply to this post by Colin Harrington
+1 first, its a never ending source of confusion for new to grails folks, most of whom seem to be surprised by this. second, if you extend the use case beyond just a controller example and use a transactional service too (which you must for any realworld transactional app) it becomes a big issue NOT to have it failOnError. Here is an example
Controller def save={ try{ myService.saveBunchOfStuff(params) redirect/render/whatever }catch(ValidationException e){ redirect/render/whatever with e.errors } } e.errors has all my errors that I can display in my transactional service method I can just do def saveBunchOfStuff(params){ ... products.save() ... widgets.save() ... skus.save() } if anything fails to save it rolls backs and throw the errors. Perfect and exactly what one would expect. it becomes a nightmare to try and standardize this across an app and monitor that is done wright with the Grails default failOnError=false. Just the mere fact of having to manually fire a Runtime or worse yet, roll back the transaction by hand if any of the saves return false? should be case enough to have failOnError default to true. On Oct 1, 2010, at 12:17 PM, Colin Harrington wrote: Jeff, |
Re: Changing the default behaviour of save()
|
|
In reply to this post by James Coleman
-1000 for "save!" unless it becomes possible without the quotes. +1 for the general idea of a variant that throws on validation failure over changing the current save() semantics.
|
Re: Changing the default behaviour of save()
|
|
In reply to this post by Jeff Brown-3
On 02/10/2010, at 2:25 AM, Jeff Brown <[hidden email]> wrote: > I expect I am the only person opposed to the change, and I am fine > with that of course. For the record... -1. ;) What is your stance on a save variant that implies failOnError? This ticks all the boxes for me as it's non breaking and allows users to use either approach. I suspect an issue is going to be coming up with a concise enough name. --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
Re: Changing the default behaviour of save()
|
|
On Fri, Oct 1, 2010 at 9:42 PM, Luke Daley <[hidden email]> wrote:
> > On 02/10/2010, at 2:25 AM, Jeff Brown <[hidden email]> wrote: > >> I expect I am the only person opposed to the change, and I am fine >> with that of course. For the record... -1. ;) > > What is your stance on a save variant that implies failOnError? > > This ticks all the boxes for me as it's non breaking and allows users to use either approach. > > I suspect an issue is going to be coming up with a concise enough name. I am in favor of a save variant that is equivalent to save(failOnError: true) because I think a lot of people want that style of programming. You can get close to that today by setting the global property that changes all calls to save() to be save(failOnError: true). That is potentially problematic if you are using any plugins that call save() and expect that to not throw an exception if validation fails. I think it is good that the option is there, but it has to be used with that in mind. I think the default behavior we have today is pretty good. I think having another method that that behaves like save(failOnError: true) is probably a good idea. My -1 vote is for changing the default behavior of save(). For me, the current behavior is the most sensible and the easiest to work with. I do acknowledge that a lot of folks don't like it. I realize that this isn't quite the same thing but it would be ridiculous if the exists() method on java.io.File threw a FileNotFoundException. Validation failures are expected, not exceptions. If one were to propose that the validate() method throw an exception I expect that would be met with greater resistance. I am only expressing a preference. I think the proposal is totally reasonable in as much as we should figure out what people want, have a discussion about that and have all of that information to work with, which is exactly what is happening. Counting the vocal is only 1 component here but that component thus far has clearly sided with the default behavior of save() being changed to throw an exception if validation fails. jb -- Jeff Brown SpringSource http://www.springsource.com/ Autism Strikes 1 in 166 Find The Cause ~ Find The Cure http://www.autismspeaks.org/ --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
|
In reply to this post by Colin Harrington
+1 for a save() variant - but... As a few people have said validation is not an exceptional circumstance, you always (should) test for invalid input. Throwing an exception to save us when we forget to check input is what we're trying to do with fail by default. Perhaps we can be cleverer about this and throw an exception when the failed save() hasn't been handled? so, and I'm not sure how we can do this, perhaps an intrinsic annotation? but... if the coder does a simple book.save() an exception is thrown when validation fails, but if the result of save is assigned, e.g. if they do: if(book.save()) {} or def result = book.save() or return book.save()
then an exception isn't thrown by default. Obvious drawback is gaining
user understanding of when an exception is thrown.That is off the top of my head, just throwing it in for discussion. Cheers, Peter. On 02/10/10 03:17, Colin Harrington wrote: Jeff, -- web: http://nerderg.com Twitter: http://twitter.com/pmcneil |
Re: Changing the default behaviour of save()
|
|
On Sat, Oct 2, 2010 at 12:39 AM, Peter McNeil <[hidden email]> wrote:
> -1 to changing the default for save() > +1 for a save() variant - but... > > As a few people have said validation is not an exceptional circumstance, you > always (should) test for invalid input. Throwing an exception to save us > when we forget to check input is what we're trying to do with fail by > default. Perhaps we can be cleverer about this and throw an exception when > the failed save() hasn't been handled? > > so, and I'm not sure how we can do this, perhaps an intrinsic annotation? > but... if the coder does a simple > > book.save() > > an exception is thrown when validation fails, but if the result of save is > assigned, e.g. if they do: > > if(book.save()) {} or def result = book.save() or return book.save() > > then an exception isn't thrown by default. Obvious drawback is gaining user > understanding of when an exception is thrown. > > That is off the top of my head, just throwing it in for discussion. > I don't think this is a good idea. However, I think it would be a great CodeNarc rule. I don't know enough about CodeNarc to know if it allows for capturing these kinds of things but if it does, then a rule to report on calls to .save() where the return value is ignored might be a reasonable thing to generate warnings about. Dealing with failOnError complicates this because in some situations one can't really know the value of the global attribute until runtime, but issuing warnings seems reasonable. For projects who want to say "yes, I know... leave me alone", of course they could turn the rule off. Just a thought. jb -- Jeff Brown SpringSource http://www.springsource.com/ Autism Strikes 1 in 166 Find The Cause ~ Find The Cure http://www.autismspeaks.org/ --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email |
|
|
In reply to this post by pledbrook
Yeah Jeff I can see a ton of holes in my suggestion, and was hoping to spark so alternative ideas. Codenarc is a good idea for this though, might have a look at that in the morning.
Jeff Brown <[hidden email]> wrote: >On Sat, Oct 2, 2010 at 12:39 AM, Peter McNeil <[hidden email]> wrote: >> -1 to changing the default for save() >> +1 for a save() variant - but... >> >> As a few people have said validation is not an exceptional circumstance, you >> always (should) test for invalid input. Throwing an exception to save us >> when we forget to check input is what we're trying to do with fail by >> default. Perhaps we can be cleverer about this and throw an exception when >> the failed save() hasn't been handled? >> >> so, and I'm not sure how we can do this, perhaps an intrinsic annotation? >> but... if the coder does a simple >> >> book.save() >> >> an exception is thrown when validation fails, but if the result of save is >> assigned, e.g. if they do: >> >> if(book.save()) {} or def result = book.save() or return book.save() >> >> then an exception isn't thrown by default. Obvious drawback is gaining user >> understanding of when an exception is thrown. >> >> That is off the top of my head, just throwing it in for discussion. >> > >I don't think this is a good idea. However, I think it would be a >great CodeNarc rule. I don't know enough about CodeNarc to know if it >allows for capturing these kinds of things but if it does, then a rule >to report on calls to .save() where the return value is ignored might >be a reasonable thing to generate warnings about. Dealing with >failOnError complicates this because in some situations one can't >really know the value of the global attribute until runtime, but >issuing warnings seems reasonable. For projects who want to say "yes, >I know... leave me alone", of course they could turn the rule off. > >Just a thought. > > > >jb >-- >Jeff Brown >SpringSource >http://www.springsource.com/ > >Autism Strikes 1 in 166 >Find The Cause ~ Find The Cure >http://www.autismspeaks.org/ > >--------------------------------------------------------------------- >To unsubscribe from this list, please visit: > > http://xircles.codehaus.org/manage_email > |
|
In reply to this post by basejump (Josh)
basejump wrote:
If changing the behaviour results in try/catch blocks wrapping every save()...yuk! I agree that any real world app must have services, currently new users are left to figure out how to interface services to controllers. The above transactional example looks good if the templates provided controllers+services by default. This would promote best practice, I currently use my own fail {} and pass maps back and forth, full set here. However I don't mind: if(widgetInstance.validate() && widgetInstance.save()) In this case a new user might expect save() to behave like so: - failOnError: true, user expected to use validate() or a service transaction and catch. - flush: true, unless inside a transaction in which case session.flush() for explicit flush. Although having said all that Grails is usually faithful to whatever Hibernate would do and once the user knows the current behaviour they can code to expect it. Cheers Gavin -- |
Re: Changing the default behaviour of save()
|
-1 for "save!"() with quotes.
+1 for save!(), especially if that notation could be used on any call to indicate that otherwise non-transactional errors should fail. +1 on changing behavior, as long as there's a simple way to detect an attempt to use it the other way. Not sure how to do that. I would prefer a simple alternate notation like save!(), coupled with a way to make one or the other be a warning based on a setting in Config.groovy. That way there's no breaking change, there's no ambiguity about the way the author intended it and there's detection if you want it. On Oct 2, 2010, at 8:07 AM, Gavin wrote:
|
«
Return to Grails - dev
|
1 view|%1 views
Loading...
| Powered by Nabble | See how NAML generates this page |
