Re: svn commit: r1401254 - /ofbiz/trunk/specialpurpose/ex
Locked
Re: svn commit: r1401254 - /ofbiz/trunk/specialpurpose/ex
 
|
Hi Adrian,
This type of pattern is pretty heavily used throughout the codebase isn't it? While updating a status history table might not be the best use of it, it does still serve as an example of that pattern. Couldn't it be fixed by including an is-not-empty condition? Thanks Scott On 24/10/2012, at 12:40 AM, [hidden email] wrote: > Author: adrianc > Date: Tue Oct 23 11:40:51 2012 > New Revision: 1401254 > > URL: http://svn.apache.org/viewvc?rev=1401254&view=rev > Log: > Commented out a bad ECA in the Example component that was causing exceptions to be thrown in the updateExample service. > > Modified: > ofbiz/trunk/specialpurpose/example/servicedef/secas.xml > > Modified: ofbiz/trunk/specialpurpose/example/servicedef/secas.xml > URL: http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/example/servicedef/secas.xml?rev=1401254&r1=1401253&r2=1401254&view=diff > ============================================================================== > --- ofbiz/trunk/specialpurpose/example/servicedef/secas.xml (original) > +++ ofbiz/trunk/specialpurpose/example/servicedef/secas.xml Tue Oct 23 11:40:51 2012 > @@ -23,8 +23,13 @@ under the License. > <eca service="createExample" event="return"> > <action service="createExampleStatus" mode="sync"/> > </eca> > + <!-- FIXME: This ECA assumes the updateExample service was called with a statusId parameter. > + The ECAs in this file demonstrate a bad design pattern. The action services should be > + invoked within the called service, not invoked by ECAs. --> > + <!-- > <eca service="updateExample" event="return"> > <condition-field field-name="statusId" operator="not-equals" to-field-name="oldStatusId"/> > <action service="createExampleStatus" mode="sync"/> > </eca> > + --> > </service-eca> > > |
Re: svn commit: r1401254 - /ofbiz/trunk/specialpurpose/ex
|
|
It could be fixed by removing the ECA condition and have the
createExampleStatus service do a better job of checking status changes. I agree the pattern is used heavily throughout the codebase, but that doesn't make it a good pattern. -Adrian On 10/23/2012 10:23 PM, Scott Gray wrote: > Hi Adrian, > > This type of pattern is pretty heavily used throughout the codebase isn't it? While updating a status history table might not be the best use of it, it does still serve as an example of that pattern. Couldn't it be fixed by including an is-not-empty condition? > > Thanks > Scott > > On 24/10/2012, at 12:40 AM, [hidden email] wrote: > >> Author: adrianc >> Date: Tue Oct 23 11:40:51 2012 >> New Revision: 1401254 >> >> URL: http://svn.apache.org/viewvc?rev=1401254&view=rev >> Log: >> Commented out a bad ECA in the Example component that was causing exceptions to be thrown in the updateExample service. >> >> Modified: >> ofbiz/trunk/specialpurpose/example/servicedef/secas.xml >> >> Modified: ofbiz/trunk/specialpurpose/example/servicedef/secas.xml >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/example/servicedef/secas.xml?rev=1401254&r1=1401253&r2=1401254&view=diff >> ============================================================================== >> --- ofbiz/trunk/specialpurpose/example/servicedef/secas.xml (original) >> +++ ofbiz/trunk/specialpurpose/example/servicedef/secas.xml Tue Oct 23 11:40:51 2012 >> @@ -23,8 +23,13 @@ under the License. >> <eca service="createExample" event="return"> >> <action service="createExampleStatus" mode="sync"/> >> </eca> >> + <!-- FIXME: This ECA assumes the updateExample service was called with a statusId parameter. >> + The ECAs in this file demonstrate a bad design pattern. The action services should be >> + invoked within the called service, not invoked by ECAs. --> >> + <!-- >> <eca service="updateExample" event="return"> >> <condition-field field-name="statusId" operator="not-equals" to-field-name="oldStatusId"/> >> <action service="createExampleStatus" mode="sync"/> >> </eca> >> + --> >> </service-eca> >> >> |
Re: svn commit: r1401254 - /ofbiz/trunk/specialpurpose/ex
|
|
Yeah that could be done but conditions exist to avoid the additional overhead of executing every possible ECA service.
The quality of the pattern aside, my concern is more about the confusion to developers. To randomly declare a bad practice in the example app for something that is heavily used throughout the system would leave me scratching my head as to how exactly I'm supposed to trigger a SECA on a specific status change. Regards Scott On 24/10/2012, at 11:09 AM, Adrian Crum wrote: > It could be fixed by removing the ECA condition and have the createExampleStatus service do a better job of checking status changes. > > I agree the pattern is used heavily throughout the codebase, but that doesn't make it a good pattern. > > -Adrian > > On 10/23/2012 10:23 PM, Scott Gray wrote: >> Hi Adrian, >> >> This type of pattern is pretty heavily used throughout the codebase isn't it? While updating a status history table might not be the best use of it, it does still serve as an example of that pattern. Couldn't it be fixed by including an is-not-empty condition? >> >> Thanks >> Scott >> >> On 24/10/2012, at 12:40 AM, [hidden email] wrote: >> >>> Author: adrianc >>> Date: Tue Oct 23 11:40:51 2012 >>> New Revision: 1401254 >>> >>> URL: http://svn.apache.org/viewvc?rev=1401254&view=rev >>> Log: >>> Commented out a bad ECA in the Example component that was causing exceptions to be thrown in the updateExample service. >>> >>> Modified: >>> ofbiz/trunk/specialpurpose/example/servicedef/secas.xml >>> >>> Modified: ofbiz/trunk/specialpurpose/example/servicedef/secas.xml >>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/example/servicedef/secas.xml?rev=1401254&r1=1401253&r2=1401254&view=diff >>> ============================================================================== >>> --- ofbiz/trunk/specialpurpose/example/servicedef/secas.xml (original) >>> +++ ofbiz/trunk/specialpurpose/example/servicedef/secas.xml Tue Oct 23 11:40:51 2012 >>> @@ -23,8 +23,13 @@ under the License. >>> <eca service="createExample" event="return"> >>> <action service="createExampleStatus" mode="sync"/> >>> </eca> >>> + <!-- FIXME: This ECA assumes the updateExample service was called with a statusId parameter. >>> + The ECAs in this file demonstrate a bad design pattern. The action services should be >>> + invoked within the called service, not invoked by ECAs. --> >>> + <!-- >>> <eca service="updateExample" event="return"> >>> <condition-field field-name="statusId" operator="not-equals" to-field-name="oldStatusId"/> >>> <action service="createExampleStatus" mode="sync"/> >>> </eca> >>> + --> >>> </service-eca> >>> >>> > |
Re: svn commit: r1401254 - /ofbiz/trunk/specialpurpose/ex
|
|
I changed the comment. There is no condition to check for not empty.
-Adrian On 10/24/2012 2:30 AM, Scott Gray wrote: > Yeah that could be done but conditions exist to avoid the additional overhead of executing every possible ECA service. > > The quality of the pattern aside, my concern is more about the confusion to developers. To randomly declare a bad practice in the example app for something that is heavily used throughout the system would leave me scratching my head as to how exactly I'm supposed to trigger a SECA on a specific status change. > > Regards > Scott > > On 24/10/2012, at 11:09 AM, Adrian Crum wrote: > >> It could be fixed by removing the ECA condition and have the createExampleStatus service do a better job of checking status changes. >> >> I agree the pattern is used heavily throughout the codebase, but that doesn't make it a good pattern. >> >> -Adrian >> >> On 10/23/2012 10:23 PM, Scott Gray wrote: >>> Hi Adrian, >>> >>> This type of pattern is pretty heavily used throughout the codebase isn't it? While updating a status history table might not be the best use of it, it does still serve as an example of that pattern. Couldn't it be fixed by including an is-not-empty condition? >>> >>> Thanks >>> Scott >>> >>> On 24/10/2012, at 12:40 AM, [hidden email] wrote: >>> >>>> Author: adrianc >>>> Date: Tue Oct 23 11:40:51 2012 >>>> New Revision: 1401254 >>>> >>>> URL: http://svn.apache.org/viewvc?rev=1401254&view=rev >>>> Log: >>>> Commented out a bad ECA in the Example component that was causing exceptions to be thrown in the updateExample service. >>>> >>>> Modified: >>>> ofbiz/trunk/specialpurpose/example/servicedef/secas.xml >>>> >>>> Modified: ofbiz/trunk/specialpurpose/example/servicedef/secas.xml >>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/example/servicedef/secas.xml?rev=1401254&r1=1401253&r2=1401254&view=diff >>>> ============================================================================== >>>> --- ofbiz/trunk/specialpurpose/example/servicedef/secas.xml (original) >>>> +++ ofbiz/trunk/specialpurpose/example/servicedef/secas.xml Tue Oct 23 11:40:51 2012 >>>> @@ -23,8 +23,13 @@ under the License. >>>> <eca service="createExample" event="return"> >>>> <action service="createExampleStatus" mode="sync"/> >>>> </eca> >>>> + <!-- FIXME: This ECA assumes the updateExample service was called with a statusId parameter. >>>> + The ECAs in this file demonstrate a bad design pattern. The action services should be >>>> + invoked within the called service, not invoked by ECAs. --> >>>> + <!-- >>>> <eca service="updateExample" event="return"> >>>> <condition-field field-name="statusId" operator="not-equals" to-field-name="oldStatusId"/> >>>> <action service="createExampleStatus" mode="sync"/> >>>> </eca> >>>> + --> >>>> </service-eca> >>>> >>>> |
«
Return to OFBiz - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |