Re: svn commit: r686377 - in /ofbiz/trunk/applications/accounting: src/org/ofbiz/accounting/ src/org/ofbiz/accounting/invoice/ src/org/ofbiz/accounting/payment/ src/org/ofbiz/accounting/thirdparty/paypal/ src/org/ofbiz/accounting/thirdparty/worldpay/ webap...

[Adam Heath-2]

[hidden email] wrote:
> Author: lektran
> Date: Fri Aug 15 14:40:30 2008
> New Revision: 686377
>
> URL: http://svn.apache.org/viewvc?rev=686377&view=rev
> Log:
> Various clean ups, no functional changes

While not having any specific feedback on this revision, I thought I
might as well mention what code changes I have been working on.

Adding generics markup(this is the biggest one).
Switching from StringBuffer to StringBuilder
Enhanced for(Iterable stuff)
Getting rid of new (Boolean|Byte|Double|Float|Integer|Long|Short)
Switching to FastList|FastMap|FastSet where appropriate
Fixing string +, to use StringBuilder, in loops

I do this, not by looking at any compiler warnings, but by editting
every single file, and looking at every line.

One thing I'd like to mention about generics; you may have noticed that
in some places, there is a Map<String, Object>, and in others, a
Map<String, ? extends Object>, and wondered what the different is.  In
the latter case, the simple explanation is that it effectively stops the
code from inserting *new* items into the map.  Ie, it makes it
read-only.  You can still remove and fetch items, however.  (if you are
interested in a more detailed explanation, just ask).

I use this to tell generic services that they should not modify the
incoming context.  It is also used when fetching a list of elements from
a DOM.

[Jacques Le Roux]

Adam,

Thanks for the last point I was not aware of that; thanks for examples of use also.

Jacques

From: "Adam Heath" <[hidden email]>

> [hidden email] wrote:
>> Author: lektran
>> Date: Fri Aug 15 14:40:30 2008
>> New Revision: 686377
>>
>> URL: http://svn.apache.org/viewvc?rev=686377&view=rev
>> Log:
>> Various clean ups, no functional changes
>
> While not having any specific feedback on this revision, I thought I
> might as well mention what code changes I have been working on.
>
> Adding generics markup(this is the biggest one).
> Switching from StringBuffer to StringBuilder
> Enhanced for(Iterable stuff)
> Getting rid of new (Boolean|Byte|Double|Float|Integer|Long|Short)
> Switching to FastList|FastMap|FastSet where appropriate
> Fixing string +, to use StringBuilder, in loops
>
> I do this, not by looking at any compiler warnings, but by editting
> every single file, and looking at every line.
>
> One thing I'd like to mention about generics; you may have noticed that
> in some places, there is a Map<String, Object>, and in others, a
> Map<String, ? extends Object>, and wondered what the different is.  In
> the latter case, the simple explanation is that it effectively stops the
> code from inserting *new* items into the map.  Ie, it makes it
> read-only.  You can still remove and fetch items, however.  (if you are
> interested in a more detailed explanation, just ask).
>
> I use this to tell generic services that they should not modify the
> incoming context.  It is also used when fetching a list of elements from
> a DOM.
>

[Adam Heath-2]

One more bug I've noticed as I've been converting the code.

I started out adding generics to framework; firstly, base.  One common
pattern in base, is several methods take a List of errorMessages; the
idea is that the code can build up the errors that it detects.

Initially, before I checked anything in, I had that list only allowing
String.  However, as I dug in further, I found that some methods not
only added String, but MessageString.  Namely, I first hit upon this
when updating the service component(ServiceUtil.java, line 219).

As I have continued to change code, I have come across instances that do
*not* handle this correctly.  They try to cast the error message to a
String.  It's only random luck that it hasn't had any problems.

[David E Jones]

On Aug 15, 2008, at 9:28 PM, Adam Heath wrote:

> One more bug I've noticed as I've been converting the code.
>
> I started out adding generics to framework; firstly, base.  One  
> common pattern in base, is several methods take a List of  
> errorMessages; the idea is that the code can build up the errors  
> that it detects.
>
> Initially, before I checked anything in, I had that list only  
> allowing String.  However, as I dug in further, I found that some  
> methods not only added String, but MessageString.  Namely, I first  
> hit upon this when updating the service component(ServiceUtil.java,  
> line 219).
>
> As I have continued to change code, I have come across instances  
> that do *not* handle this correctly.  They try to cast the error  
> message to a String.  It's only random luck that it hasn't had any  
> problems.

Ideally the MessageString class would extend String, but if I remember  
right that wasn't allowed (don't know if it is now or not in Java 5).

-David

[Adrian Crum-2]

--- On Sat, 8/16/08, David E Jones <[hidden email]> wrote:
> Ideally the MessageString class would extend String, but if
> I remember  
> right that wasn't allowed (don't know if it is now
> or not in Java 5).

The String class is final, and I'm sure it always will be - http://java.sun.com/j2se/1.5.0/docs/api/java/lang/String.html. Subclassing the String class would open up a huge security hole.

-Adrian