You are viewing a plain text version of this content. The canonical link for it is here.
Posted to java-dev@axis.apache.org by Sanjiva Weerawarana <sa...@opensource.lk> on 2007/02/14 16:26:06 UTC

Re: [AXIS2] Input Parameter Validation, Error and Exception handling

Please submit a patch .. along with proper indentation ;-).

Sanjiva.

On Wed, 2007-02-14 at 16:32 +0530, Sameera Madushan wrote:
> I've also found out some places in axis2 code which can have a
> negative impact on the performance. For an example consider the
> following code in the AbstractMessageReceiver class.
> 
> if (service.getParameter(Constants.SERVICE_OBJECT_SUPPLIER) != null) {
>                 Parameter serviceObjectParam =
>                         service.getParameter(Constants.SERVICE_OBJECT_SUPPLIER);
>                 Class serviceObjectMaker =
> Loader.loadClass(classLoader, ((String)
>                         serviceObjectParam.getValue()).trim());
> .....
> 
> This can be refactored as,
> 
> Parameter serviceObjectParam =
>                         service.getParameter(Constants.SERVICE_OBJECT_SUPPLIER);
> if (serviceObjectParam != null) {
>                       Class serviceObjectMaker =
> Loader.loadClass(classLoader, ((String)
>                         serviceObjectParam.getValue()).trim());
> ..........
> 
> ASAIK the latter removes the duplicate method invocation. In this
> instance, invoking "getParameter" method result in searching a
> HashMap. Duplicating these heavy operations is not a good idea.
> 
> On 2/14/07, Sanjaya Karunasena <sa...@wso2.com> wrote:
> > Hi David,
> >
> > I din't mean to propose some hard constraints or major refactoring activity.
> > Problems I have noticed are very natural in an open/large development
> > environment. Having some guidelines, discussions on such topics will help
> > every one to come to the same page and minimize the pain in the future (IMO).
> >
> > Please find my comments below.
> >
> > Regards,
> > Sanjaya
> >
> > Note: BTW, I forgot to put the prefix [AXIS2] in the subject earlier. Sorry
> > about that.
> >
> > On Tuesday 13 February 2007 17:40, David Illsley wrote:
> > > Hi Sanjaya,
> > > If you're suggesting a mass change based on a set of rules, I'm not
> > > enthusiastic (and quite likely to -1 it)... it's quite likely that
> > > some of the seemingly extraneous checks are there because there have
> > > been problems in the past.
> > >
> > Not at all. But when we do defect fixes and other improvements if we can fix
> > up some of these without having to modify lot of code, is I am proposing.
> > Last thing you want is some nice looking code with broken functionality.
> >
> > > Please list the methods which you believe are being over-cautious and
> > > we can address them individually with a little thought.
> > >
> > I will add the areas I came across as improvements to JIRA and where possible
> > a patch as well. So that you all can make the call.
> >
> > > If you're looking for guidelines for future contributions then I'm
> > > open to developing them (I'm not aware that they exist now) but don't
> > > wish to be tightly constrained to them. I tend to write cautious code,
> > > and unless there's a clear, specific performance problem, that's how
> > > I'd like to continue.
> > >
> > I think we are on the same page here.
> >
> > > David
> > >
> > > On 13/02/07, Sanjaya Karunasena <sa...@wso2.com> wrote:
> > > > Hi All,
> > > >
> > > > After going through some of the Axis2 code, I have noticed many
> > > > instances of duplicate input parameter validations leading to redundant
> > > > code. This has negative impact on performance as well. Also the error and
> > > > exception handling policy is not clear, or may be not consistent.
> > > >
> > > > What are the typical guidelines followed in the project?
> > > >
> > > >
> > > > Few suggestions on input parameter validation are;
> > > > * An instance of a class which make use of an input parameter for the
> > > > first time should validate the parameter
> > > > * A instance of a interface class (Actor interface) should assume
> > > > default values for required (mandatory) parameters where possible (Could
> > > > be due to a flexibility provided in a specification)
> > > > * If there are optional parameters for an interface, that should be
> > > > handled through method overloading (not by allowing null inputs)
> > > > * Any exceptions to this can be discussed and come to some logical
> > > > agreement based on the specific scenario (80, 20 rule)
> > > >
> > > >
> > > > Thanks
> > > > Sanjaya
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
> > > > For additional commands, e-mail: axis-dev-help@ws.apache.org
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
> > For additional commands, e-mail: axis-dev-help@ws.apache.org
> >
> >
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
> For additional commands, e-mail: axis-dev-help@ws.apache.org
> 
-- 
Sanjiva Weerawarana, Ph.D.
Founder & Director; Lanka Software Foundation; http://www.opensource.lk/
Founder, Chairman & CEO; WSO2, Inc.; http://www.wso2.com/
Director; Open Source Initiative; http://www.opensource.org/
Member; Apache Software Foundation; http://www.apache.org/
Visiting Lecturer; University of Moratuwa; http://www.cse.mrt.ac.lk/


---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org


Re: [AXIS2] Input Parameter Validation, Error and Exception handling

Posted by Eran Chinthaka <ch...@opensource.lk>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Sanjaya,

We don't have lot of rules for code formatting. Here are the bare
minimum that I use.

*Column width*  - 100 (no fights again on this please :))
*Tab size (indentation)*   - 4 spaces (NO tabs)
*Imports*       - * should not be used as much as possible (my setting
is to use *, only if I import more than 100 classes from the same
package, which is highly unlikely)
*Apache License Header* - This must be present in each and every file.
No author tags ( Dims will take care of it :) )

- -- Chinthaka

Sanjaya Karunasena wrote:
> I was under the impression there is a code formatting tool which get executed 
> as part of the build. Isn't that the case? Since every developer runs a local 
> build with the latest from the SVN before committing or submitting a patch, 
> the code will be automatically formatted.
> 
> Also it will be helpful for newbies, if committers who are using different 
> IDEs can export their code formatting settings and have them available to 
> download. Then newbies can simply import them to the project they are working 
> on.
> 
> Sanjaya
> 
> On Wednesday 14 February 2007 20:56, Sanjiva Weerawarana wrote:
>> Please submit a patch .. along with proper indentation ;-).
>>
>> Sanjiva.
>>
>> On Wed, 2007-02-14 at 16:32 +0530, Sameera Madushan wrote:
>>> I've also found out some places in axis2 code which can have a
>>> negative impact on the performance. For an example consider the
>>> following code in the AbstractMessageReceiver class.
>>>
>>> if (service.getParameter(Constants.SERVICE_OBJECT_SUPPLIER) != null) {
>>>                 Parameter serviceObjectParam =
>>>                        
>>> service.getParameter(Constants.SERVICE_OBJECT_SUPPLIER); Class
>>> serviceObjectMaker =
>>> Loader.loadClass(classLoader, ((String)
>>>                         serviceObjectParam.getValue()).trim());
>>> .....
>>>
>>> This can be refactored as,
>>>
>>> Parameter serviceObjectParam =
>>>                        
>>> service.getParameter(Constants.SERVICE_OBJECT_SUPPLIER); if
>>> (serviceObjectParam != null) {
>>>                       Class serviceObjectMaker =
>>> Loader.loadClass(classLoader, ((String)
>>>                         serviceObjectParam.getValue()).trim());
>>> ..........
>>>
>>> ASAIK the latter removes the duplicate method invocation. In this
>>> instance, invoking "getParameter" method result in searching a
>>> HashMap. Duplicating these heavy operations is not a good idea.
>>>
>>> On 2/14/07, Sanjaya Karunasena <sa...@wso2.com> wrote:
>>>> Hi David,
>>>>
>>>> I din't mean to propose some hard constraints or major refactoring
>>>> activity. Problems I have noticed are very natural in an open/large
>>>> development environment. Having some guidelines, discussions on such
>>>> topics will help every one to come to the same page and minimize the
>>>> pain in the future (IMO).
>>>>
>>>> Please find my comments below.
>>>>
>>>> Regards,
>>>> Sanjaya
>>>>
>>>> Note: BTW, I forgot to put the prefix [AXIS2] in the subject earlier.
>>>> Sorry about that.
>>>>
>>>> On Tuesday 13 February 2007 17:40, David Illsley wrote:
>>>>> Hi Sanjaya,
>>>>> If you're suggesting a mass change based on a set of rules, I'm not
>>>>> enthusiastic (and quite likely to -1 it)... it's quite likely that
>>>>> some of the seemingly extraneous checks are there because there have
>>>>> been problems in the past.
>>>> Not at all. But when we do defect fixes and other improvements if we
>>>> can fix up some of these without having to modify lot of code, is I am
>>>> proposing. Last thing you want is some nice looking code with broken
>>>> functionality.
>>>>
>>>>> Please list the methods which you believe are being over-cautious and
>>>>> we can address them individually with a little thought.
>>>> I will add the areas I came across as improvements to JIRA and where
>>>> possible a patch as well. So that you all can make the call.
>>>>
>>>>> If you're looking for guidelines for future contributions then I'm
>>>>> open to developing them (I'm not aware that they exist now) but don't
>>>>> wish to be tightly constrained to them. I tend to write cautious
>>>>> code, and unless there's a clear, specific performance problem,
>>>>> that's how I'd like to continue.
>>>> I think we are on the same page here.
>>>>
>>>>> David
>>>>>
>>>>> On 13/02/07, Sanjaya Karunasena <sa...@wso2.com> wrote:
>>>>>> Hi All,
>>>>>>
>>>>>> After going through some of the Axis2 code, I have noticed many
>>>>>> instances of duplicate input parameter validations leading to
>>>>>> redundant code. This has negative impact on performance as well.
>>>>>> Also the error and exception handling policy is not clear, or may
>>>>>> be not consistent.
>>>>>>
>>>>>> What are the typical guidelines followed in the project?
>>>>>>
>>>>>>
>>>>>> Few suggestions on input parameter validation are;
>>>>>> * An instance of a class which make use of an input parameter for
>>>>>> the first time should validate the parameter
>>>>>> * A instance of a interface class (Actor interface) should assume
>>>>>> default values for required (mandatory) parameters where possible
>>>>>> (Could be due to a flexibility provided in a specification)
>>>>>> * If there are optional parameters for an interface, that should be
>>>>>> handled through method overloading (not by allowing null inputs)
>>>>>> * Any exceptions to this can be discussed and come to some logical
>>>>>> agreement based on the specific scenario (80, 20 rule)
>>>>>>
>>>>>>
>>>>>> Thanks
>>>>>> Sanjaya
>>>>>>
>>>>>> -------------------------------------------------------------------
>>>>>> -- To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org For
>>>>>> additional commands, e-mail: axis-dev-help@ws.apache.org
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
>>>> For additional commands, e-mail: axis-dev-help@ws.apache.org
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
>>> For additional commands, e-mail: axis-dev-help@ws.apache.org
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
> For additional commands, e-mail: axis-dev-help@ws.apache.org
> 
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFF1+zWjON2uBzUhh8RAnddAKCFSlXhm1mAjaZs/nIDzZW28npt0wCfYBt1
v0Q4Hl+gDgCWVTQek58aTIk=
=ygUa
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org


Re: [AXIS2] Input Parameter Validation, Error and Exception handling

Posted by Sanjaya Karunasena <sa...@wso2.com>.
I was under the impression there is a code formatting tool which get executed 
as part of the build. Isn't that the case? Since every developer runs a local 
build with the latest from the SVN before committing or submitting a patch, 
the code will be automatically formatted.

Also it will be helpful for newbies, if committers who are using different 
IDEs can export their code formatting settings and have them available to 
download. Then newbies can simply import them to the project they are working 
on.

Sanjaya

On Wednesday 14 February 2007 20:56, Sanjiva Weerawarana wrote:
> Please submit a patch .. along with proper indentation ;-).
>
> Sanjiva.
>
> On Wed, 2007-02-14 at 16:32 +0530, Sameera Madushan wrote:
> > I've also found out some places in axis2 code which can have a
> > negative impact on the performance. For an example consider the
> > following code in the AbstractMessageReceiver class.
> >
> > if (service.getParameter(Constants.SERVICE_OBJECT_SUPPLIER) != null) {
> >                 Parameter serviceObjectParam =
> >                        
> > service.getParameter(Constants.SERVICE_OBJECT_SUPPLIER); Class
> > serviceObjectMaker =
> > Loader.loadClass(classLoader, ((String)
> >                         serviceObjectParam.getValue()).trim());
> > .....
> >
> > This can be refactored as,
> >
> > Parameter serviceObjectParam =
> >                        
> > service.getParameter(Constants.SERVICE_OBJECT_SUPPLIER); if
> > (serviceObjectParam != null) {
> >                       Class serviceObjectMaker =
> > Loader.loadClass(classLoader, ((String)
> >                         serviceObjectParam.getValue()).trim());
> > ..........
> >
> > ASAIK the latter removes the duplicate method invocation. In this
> > instance, invoking "getParameter" method result in searching a
> > HashMap. Duplicating these heavy operations is not a good idea.
> >
> > On 2/14/07, Sanjaya Karunasena <sa...@wso2.com> wrote:
> > > Hi David,
> > >
> > > I din't mean to propose some hard constraints or major refactoring
> > > activity. Problems I have noticed are very natural in an open/large
> > > development environment. Having some guidelines, discussions on such
> > > topics will help every one to come to the same page and minimize the
> > > pain in the future (IMO).
> > >
> > > Please find my comments below.
> > >
> > > Regards,
> > > Sanjaya
> > >
> > > Note: BTW, I forgot to put the prefix [AXIS2] in the subject earlier.
> > > Sorry about that.
> > >
> > > On Tuesday 13 February 2007 17:40, David Illsley wrote:
> > > > Hi Sanjaya,
> > > > If you're suggesting a mass change based on a set of rules, I'm not
> > > > enthusiastic (and quite likely to -1 it)... it's quite likely that
> > > > some of the seemingly extraneous checks are there because there have
> > > > been problems in the past.
> > >
> > > Not at all. But when we do defect fixes and other improvements if we
> > > can fix up some of these without having to modify lot of code, is I am
> > > proposing. Last thing you want is some nice looking code with broken
> > > functionality.
> > >
> > > > Please list the methods which you believe are being over-cautious and
> > > > we can address them individually with a little thought.
> > >
> > > I will add the areas I came across as improvements to JIRA and where
> > > possible a patch as well. So that you all can make the call.
> > >
> > > > If you're looking for guidelines for future contributions then I'm
> > > > open to developing them (I'm not aware that they exist now) but don't
> > > > wish to be tightly constrained to them. I tend to write cautious
> > > > code, and unless there's a clear, specific performance problem,
> > > > that's how I'd like to continue.
> > >
> > > I think we are on the same page here.
> > >
> > > > David
> > > >
> > > > On 13/02/07, Sanjaya Karunasena <sa...@wso2.com> wrote:
> > > > > Hi All,
> > > > >
> > > > > After going through some of the Axis2 code, I have noticed many
> > > > > instances of duplicate input parameter validations leading to
> > > > > redundant code. This has negative impact on performance as well.
> > > > > Also the error and exception handling policy is not clear, or may
> > > > > be not consistent.
> > > > >
> > > > > What are the typical guidelines followed in the project?
> > > > >
> > > > >
> > > > > Few suggestions on input parameter validation are;
> > > > > * An instance of a class which make use of an input parameter for
> > > > > the first time should validate the parameter
> > > > > * A instance of a interface class (Actor interface) should assume
> > > > > default values for required (mandatory) parameters where possible
> > > > > (Could be due to a flexibility provided in a specification)
> > > > > * If there are optional parameters for an interface, that should be
> > > > > handled through method overloading (not by allowing null inputs)
> > > > > * Any exceptions to this can be discussed and come to some logical
> > > > > agreement based on the specific scenario (80, 20 rule)
> > > > >
> > > > >
> > > > > Thanks
> > > > > Sanjaya
> > > > >
> > > > > -------------------------------------------------------------------
> > > > >-- To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org For
> > > > > additional commands, e-mail: axis-dev-help@ws.apache.org
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
> > > For additional commands, e-mail: axis-dev-help@ws.apache.org
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
> > For additional commands, e-mail: axis-dev-help@ws.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org