You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commons-dev@ws.apache.org by Glen Daniels <gl...@thoughtcraft.com> on 2007/03/07 03:35:27 UTC

[axiom] [axis2] Coding style

Hey folks:

Quick code commentary....

Here's some code starting at line 85 in SOAP12FaultImpl:

     public void setReason(SOAPFaultReason reason) throws 
SOAPProcessingException {
         if (!(reason instanceof SOAP12FaultReasonImpl)) {
             throw new SOAPProcessingException(
                     "Expecting SOAP 1.2 implementation of SOAP Fault 
Reason. " +
                     "But received some other implementation");
         }
         super.setReason(reason);
     }

Two comments here.

1) Messages should be internationalized and accessed via resource APIs. 
  Really.  At some point we should all stop working on features and take 
a few days to hang out and make a serious push to do this across the 
board.  After that's done we should implement a daily automated scan 
(like Axis1 does on each build) across the source to alert us of 
non-internationalized messages.

2) When generating exceptions like this, it really isn't very useful to 
say "received some other implementation" or "value wasn't what was 
expected".  It's much more helpful to actually put the problem value 
into the exception.... In this case it only takes an extra moment to code

    "but received " + reason.getClass() + "."

    and the more informative results, when you are trying to debug 
something a year and a half later, will be well worth the time.  In 
general our fault messages are not informative enough and we need to do 
better about helping both the developers and the end users.

Clearly this particular example isn't any kind of big deal on its own, 
but we have this kind of thing all over the place and we should a) begin 
working on cleaning it up, and b) be a little more careful when writing 
new code.

We should consider adding these kinds of things to our coding style 
guidelines, IMO.

Thanks,
--Glen

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


Re: [axiom] [axis2] Coding style

Posted by Alexander Veit <li...@nezwerg.de>.
Hi Glen,

Glen Daniels wrote:
> 1) Messages should be internationalized and accessed via resource APIs. 
>  Really.  At some point we should all stop working on features and take 
> a few days to hang out and make a serious push to do this across the 
> board.  After that's done we should implement a daily automated scan 
> (like Axis1 does on each build) across the source to alert us of 
> non-internationalized messages.

In my opinion, there are a some problems with internationalized 
exception messages:

- Confusion of tongues. While error notifications for end users 
certainly should be localized, exeption messages that normally appear in 
application logs and like that shouldn't, imo. If similar problems 
cannot neccessarily be identified by similar error messages, getting 
support for a specific problem becomes difficult.

- Error messages that are accessed via resource keys tend to become lies 
over time. Resource key reuse, copy&paste, non-adaption to modified 
code, or ambiguous resource key names do their corrosive work here;-)

- Error messages that are accessed via resource keys tend to be static. 
I absolutely agree with Sanjiva ("... give all the potentially useful 
info to the developer..."), so including information that's only 
available at runtime is often a good idea. Creating such dynamic error 
messages for different languages is difficult.


-- 
Just my 2 cents,
Alex

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


Re: [axiom] [axis2] Coding style

Posted by Ajith Ranabahu <aj...@gmail.com>.
Hi all,

>
> LOL :)  Seriously, though, someone probably does need to own the
> structural part (making sure that we have common utilities for getting
> messages and finding resource files).  The actual work of moving the
> messages into resource files and changing the code should be a task that
> we split up and everyone shares... ApacheCon EU's hackathon might be a
> good time for this kind of thing....
>

We do have this framework in place. However rather than keeping
everything in one place we made i18n sections in each module so that
each module has its own lang property file (I'm guessing we probably
did that after moving out Axiom). If you look at the source trees of
the modules kernel, codegen, adb-codegen etc, the i18n package
includes the language property files.

One drawback in the way we have right now is the copying of the
resources class. I believe that each module should have its own lang
file (surely it would be impractical to maintain a huge property file
with every string used in every class in every module!) but perhaps
the resource loading class can be maintained separately in one place ?
Right now its all static.


> >> 2) When generating exceptions like this, it really isn't very useful to
> >> say "received some other implementation" or "value wasn't what was
> >> expected".  It's much more helpful to actually put the problem value
> >> into the exception.... In this case it only takes an extra moment to code
> >>
> >>    "but received " + reason.getClass() + "."
> >
> > 1000 appologies for this code written by me some time back. Won't happen
> > again :(.
>
> Don't take it personally (I was hoping for a ":)" instead of a ":("), we
> really do have these kinds of issues everywhere and we ALL do this stuff
> sometimes.  I just happened to notice this a bunch of places and think
> it's useful to remind ourselves of patterns.
>
> --Glen
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
> For additional commands, e-mail: axis-dev-help@ws.apache.org
>
>


-- 
Ajith Ranabahu

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


Re: [axiom] [axis2] Coding style

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


> LOL :)  Seriously, though, someone probably does need to own the
> structural part (making sure that we have common utilities for getting
> messages and finding resource files).  The actual work of moving the
> messages into resource files and changing the code should be a task that
> we split up and everyone shares... ApacheCon EU's hackathon might be a
> good time for this kind of thing....

I am sorry as I won't be able to help in this process as I won't be
coming to ApacheCon EU. But Deepal will be coming and I hope he will be
able to help you.

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

iD8DBQFF7jT8jON2uBzUhh8RAqHVAKCT5WtWakQJ4sKlaVKLqE6SOGzchQCbBR4m
FdVYg7pntDW+Qd1teBGVN8s=
=EqQL
-----END PGP SIGNATURE-----

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


Re: [axiom] [axis2] Coding style

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


> LOL :)  Seriously, though, someone probably does need to own the
> structural part (making sure that we have common utilities for getting
> messages and finding resource files).  The actual work of moving the
> messages into resource files and changing the code should be a task that
> we split up and everyone shares... ApacheCon EU's hackathon might be a
> good time for this kind of thing....

I am sorry as I won't be able to help in this process as I won't be
coming to ApacheCon EU. But Deepal will be coming and I hope he will be
able to help you.

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

iD8DBQFF7jT8jON2uBzUhh8RAqHVAKCT5WtWakQJ4sKlaVKLqE6SOGzchQCbBR4m
FdVYg7pntDW+Qd1teBGVN8s=
=EqQL
-----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: [axiom] [axis2] Coding style

Posted by Glen Daniels <gl...@thoughtcraft.com>.
Hi Chinthaka!

>> 1) Messages should be internationalized and accessed via resource APIs.
>>  Really.  At some point we should all stop working on features and take
>> a few days to hang out and make a serious push to do this across the
>> board.  After that's done we should implement a daily automated scan
>> (like Axis1 does on each build) across the source to alert us of
>> non-internationalized messages.
> 
> Glen, thanks for owning the process of I18N'ing everything. This was in
> our todo list for a long time but no one had a chance to do it. I hope
> you are the best person for that ;)

LOL :)  Seriously, though, someone probably does need to own the 
structural part (making sure that we have common utilities for getting 
messages and finding resource files).  The actual work of moving the 
messages into resource files and changing the code should be a task that 
we split up and everyone shares... ApacheCon EU's hackathon might be a 
good time for this kind of thing....

>> 2) When generating exceptions like this, it really isn't very useful to
>> say "received some other implementation" or "value wasn't what was
>> expected".  It's much more helpful to actually put the problem value
>> into the exception.... In this case it only takes an extra moment to code
>>
>>    "but received " + reason.getClass() + "."
> 
> 1000 appologies for this code written by me some time back. Won't happen
> again :(.

Don't take it personally (I was hoping for a ":)" instead of a ":("), we 
really do have these kinds of issues everywhere and we ALL do this stuff 
sometimes.  I just happened to notice this a bunch of places and think 
it's useful to remind ourselves of patterns.

--Glen

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


Re: [axiom] [axis2] Coding style

Posted by Sanjiva Weerawarana <sa...@opensource.lk>.
Eran Chinthaka wrote:
>> 2) When generating exceptions like this, it really isn't very useful to
>> say "received some other implementation" or "value wasn't what was
>> expected".  It's much more helpful to actually put the problem value
>> into the exception.... In this case it only takes an extra moment to code
>>
>>    "but received " + reason.getClass() + "."
> 
> 
> 1000 appologies for this code written by me some time back. Won't happen
> again :(.

Its not about the code you wrote at all .. its about a pattern of 
development that *EVERYONE* should follow: when throwing an exception give 
all the potentially useful info to the developer. Do not swallow any of it.

Sanjiva.
-- 
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: commons-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: commons-dev-help@ws.apache.org


Re: [axiom] [axis2] Coding style

Posted by Glen Daniels <gl...@thoughtcraft.com>.
Hi Chinthaka!

>> 1) Messages should be internationalized and accessed via resource APIs.
>>  Really.  At some point we should all stop working on features and take
>> a few days to hang out and make a serious push to do this across the
>> board.  After that's done we should implement a daily automated scan
>> (like Axis1 does on each build) across the source to alert us of
>> non-internationalized messages.
> 
> Glen, thanks for owning the process of I18N'ing everything. This was in
> our todo list for a long time but no one had a chance to do it. I hope
> you are the best person for that ;)

LOL :)  Seriously, though, someone probably does need to own the 
structural part (making sure that we have common utilities for getting 
messages and finding resource files).  The actual work of moving the 
messages into resource files and changing the code should be a task that 
we split up and everyone shares... ApacheCon EU's hackathon might be a 
good time for this kind of thing....

>> 2) When generating exceptions like this, it really isn't very useful to
>> say "received some other implementation" or "value wasn't what was
>> expected".  It's much more helpful to actually put the problem value
>> into the exception.... In this case it only takes an extra moment to code
>>
>>    "but received " + reason.getClass() + "."
> 
> 1000 appologies for this code written by me some time back. Won't happen
> again :(.

Don't take it personally (I was hoping for a ":)" instead of a ":("), we 
really do have these kinds of issues everywhere and we ALL do this stuff 
sometimes.  I just happened to notice this a bunch of places and think 
it's useful to remind ourselves of patterns.

--Glen

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


Re: [axiom] [axis2] Coding style

Posted by Sanjiva Weerawarana <sa...@opensource.lk>.
Eran Chinthaka wrote:
>> 2) When generating exceptions like this, it really isn't very useful to
>> say "received some other implementation" or "value wasn't what was
>> expected".  It's much more helpful to actually put the problem value
>> into the exception.... In this case it only takes an extra moment to code
>>
>>    "but received " + reason.getClass() + "."
> 
> 
> 1000 appologies for this code written by me some time back. Won't happen
> again :(.

Its not about the code you wrote at all .. its about a pattern of 
development that *EVERYONE* should follow: when throwing an exception give 
all the potentially useful info to the developer. Do not swallow any of it.

Sanjiva.
-- 
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: [axiom] [axis2] Coding style

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

Glen Daniels wrote:
> Hey folks:
> 
> Quick code commentary....
> 
> Here's some code starting at line 85 in SOAP12FaultImpl:
> 
>     public void setReason(SOAPFaultReason reason) throws
> SOAPProcessingException {
>         if (!(reason instanceof SOAP12FaultReasonImpl)) {
>             throw new SOAPProcessingException(
>                     "Expecting SOAP 1.2 implementation of SOAP Fault
> Reason. " +
>                     "But received some other implementation");
>         }
>         super.setReason(reason);
>     }
> 
> Two comments here.
> 
> 1) Messages should be internationalized and accessed via resource APIs.
>  Really.  At some point we should all stop working on features and take
> a few days to hang out and make a serious push to do this across the
> board.  After that's done we should implement a daily automated scan
> (like Axis1 does on each build) across the source to alert us of
> non-internationalized messages.

Glen, thanks for owning the process of I18N'ing everything. This was in
our todo list for a long time but no one had a chance to do it. I hope
you are the best person for that ;)

> 
> 2) When generating exceptions like this, it really isn't very useful to
> say "received some other implementation" or "value wasn't what was
> expected".  It's much more helpful to actually put the problem value
> into the exception.... In this case it only takes an extra moment to code
> 
>    "but received " + reason.getClass() + "."


1000 appologies for this code written by me some time back. Won't happen
again :(.

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

iD8DBQFF7icIjON2uBzUhh8RAhPmAJ4xV0Df9D9shgIzXDoer/lFZLbDTACdFIr1
S0JkiJs+VFx59poGIALOIxU=
=gY6q
-----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: [axiom] [axis2] Coding style

Posted by Sanjiva Weerawarana <sa...@opensource.lk>.
Glen Daniels wrote:
> Two comments here.
> 
> 1) Messages should be internationalized and accessed via resource APIs. 
>  Really.  At some point we should all stop working on features and take 
> a few days to hang out and make a serious push to do this across the 
> board.  After that's done we should implement a daily automated scan 
> (like Axis1 does on each build) across the source to alert us of 
> non-internationalized messages.

+1 .. I thought we had the framework in place for this but if not AC EU 
would be a great time to get the framework in place. I thought IBM folks 
had committed something for this but I could be wrong.

Fixing each message is going to be an on-going process.

> 2) When generating exceptions like this, it really isn't very useful to 
> say "received some other implementation" or "value wasn't what was 
> expected".  It's much more helpful to actually put the problem value 
> into the exception.... In this case it only takes an extra moment to code
> 
>    "but received " + reason.getClass() + "."
> 
>    and the more informative results, when you are trying to debug 
> something a year and a half later, will be well worth the time.  In 
> general our fault messages are not informative enough and we need to do 
> better about helping both the developers and the end users.

Definitely +1!

> Clearly this particular example isn't any kind of big deal on its own, 
> but we have this kind of thing all over the place and we should a) begin 
> working on cleaning it up, and b) be a little more careful when writing 
> new code.

+1 for getting used to this for all new exceptions.

> We should consider adding these kinds of things to our coding style 
> guidelines, IMO.

+1 to add it!

Sanjiva.
-- 
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: [axiom] [axis2] Coding style

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

Glen Daniels wrote:
> Hey folks:
> 
> Quick code commentary....
> 
> Here's some code starting at line 85 in SOAP12FaultImpl:
> 
>     public void setReason(SOAPFaultReason reason) throws
> SOAPProcessingException {
>         if (!(reason instanceof SOAP12FaultReasonImpl)) {
>             throw new SOAPProcessingException(
>                     "Expecting SOAP 1.2 implementation of SOAP Fault
> Reason. " +
>                     "But received some other implementation");
>         }
>         super.setReason(reason);
>     }
> 
> Two comments here.
> 
> 1) Messages should be internationalized and accessed via resource APIs.
>  Really.  At some point we should all stop working on features and take
> a few days to hang out and make a serious push to do this across the
> board.  After that's done we should implement a daily automated scan
> (like Axis1 does on each build) across the source to alert us of
> non-internationalized messages.

Glen, thanks for owning the process of I18N'ing everything. This was in
our todo list for a long time but no one had a chance to do it. I hope
you are the best person for that ;)

> 
> 2) When generating exceptions like this, it really isn't very useful to
> say "received some other implementation" or "value wasn't what was
> expected".  It's much more helpful to actually put the problem value
> into the exception.... In this case it only takes an extra moment to code
> 
>    "but received " + reason.getClass() + "."


1000 appologies for this code written by me some time back. Won't happen
again :(.

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

iD8DBQFF7icIjON2uBzUhh8RAhPmAJ4xV0Df9D9shgIzXDoer/lFZLbDTACdFIr1
S0JkiJs+VFx59poGIALOIxU=
=gY6q
-----END PGP SIGNATURE-----

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


Re: [axiom] [axis2] Coding style

Posted by Sanjiva Weerawarana <sa...@opensource.lk>.
Glen Daniels wrote:
> Two comments here.
> 
> 1) Messages should be internationalized and accessed via resource APIs. 
>  Really.  At some point we should all stop working on features and take 
> a few days to hang out and make a serious push to do this across the 
> board.  After that's done we should implement a daily automated scan 
> (like Axis1 does on each build) across the source to alert us of 
> non-internationalized messages.

+1 .. I thought we had the framework in place for this but if not AC EU 
would be a great time to get the framework in place. I thought IBM folks 
had committed something for this but I could be wrong.

Fixing each message is going to be an on-going process.

> 2) When generating exceptions like this, it really isn't very useful to 
> say "received some other implementation" or "value wasn't what was 
> expected".  It's much more helpful to actually put the problem value 
> into the exception.... In this case it only takes an extra moment to code
> 
>    "but received " + reason.getClass() + "."
> 
>    and the more informative results, when you are trying to debug 
> something a year and a half later, will be well worth the time.  In 
> general our fault messages are not informative enough and we need to do 
> better about helping both the developers and the end users.

Definitely +1!

> Clearly this particular example isn't any kind of big deal on its own, 
> but we have this kind of thing all over the place and we should a) begin 
> working on cleaning it up, and b) be a little more careful when writing 
> new code.

+1 for getting used to this for all new exceptions.

> We should consider adding these kinds of things to our coding style 
> guidelines, IMO.

+1 to add it!

Sanjiva.
-- 
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: commons-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: commons-dev-help@ws.apache.org