Re: svn commit: r751220 - /ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java

[Adam Heath-2]

[hidden email] wrote:

> Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java?rev=751220&r1=751219&r2=751220&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java (original)
> +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java Sat Mar  7 08:12:32 2009
> @@ -110,7 +110,7 @@
>  
>              writer.append('>');
>              
> -            if(UtilValidate.isNotEmpty(request.getAttribute("image"))){
> +            if (UtilValidate.isNotEmpty(request.getAttribute("image"))){
>                  writer.append("<img src = \""+request.getAttribute("image").toString()+"\"/>");
>              }

This is unrelated, but doing StringBuilder.append("abc"+123) is wrong.
 You get double StringBuilder instantations.

Also, the spaces around the = in the html should be removed.

[Jacques Le Roux]

I agree, hopefully Hans will read you :o)
http://svn.apache.org/viewvc?view=rev&revision=745434

Jacques

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

> [hidden email] wrote:
>> Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java?rev=751220&r1=751219&r2=751220&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java (original)
>> +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java Sat Mar  7 08:12:32 2009
>> @@ -110,7 +110,7 @@
>>
>>              writer.append('>');
>>
>> -            if(UtilValidate.isNotEmpty(request.getAttribute("image"))){
>> +            if (UtilValidate.isNotEmpty(request.getAttribute("image"))){
>>                  writer.append("<img src = \""+request.getAttribute("image").toString()+"\"/>");
>>              }
>
> This is unrelated, but doing StringBuilder.append("abc"+123) is wrong.
> You get double StringBuilder instantations.
>
> Also, the spaces around the = in the html should be removed.
>

[Adrian Crum-2]

Wouldn't that create two String instances, then pass the final String result to StringBuilder.append()?

-Adrian

--- On Sat, 3/7/09, Adam Heath <[hidden email]> wrote:

> From: Adam Heath <[hidden email]>
> Subject: Re: svn commit: r751220 - /ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
> To: [hidden email]
> Date: Saturday, March 7, 2009, 9:39 AM
> [hidden email] wrote:
> > Modified:
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
> > URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java?rev=751220&r1=751219&r2=751220&view=diff
> >
> ==============================================================================
> > ---
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
> (original)
> > +++
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
> Sat Mar  7 08:12:32 2009
> > @@ -110,7 +110,7 @@
> >  
> >              writer.append('>');
> >              
> > -          
> if(UtilValidate.isNotEmpty(request.getAttribute("image"))){
> > +            if
> (UtilValidate.isNotEmpty(request.getAttribute("image"))){
> >                  writer.append("<img src =
> \""+request.getAttribute("image").toString()+"\"/>");
> >              }
>
> This is unrelated, but doing
> StringBuilder.append("abc"+123) is wrong.
>  You get double StringBuilder instantations.
>
> Also, the spaces around the = in the html should be
> removed.

     

[Adam Heath-2]

Adrian Crum wrote:
> Wouldn't that create two String instances, then pass the final String result to StringBuilder.append()?

I don't understand what you are saying.  The way the code currently
is, *does* create a temporary StringBuilder, and a new temporary
String, which then gets passed to the outer StringBuilder writer instance.

My suggestion was to not do it this way; are you agreeing with that,
or saying something else?

[Jacques Le Roux]

Anyway, Hans fixed it at r751348

Jacques

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

> Adrian Crum wrote:
>> Wouldn't that create two String instances, then pass the final String result to StringBuilder.append()?
>
> I don't understand what you are saying.  The way the code currently
> is, *does* create a temporary StringBuilder, and a new temporary
> String, which then gets passed to the outer StringBuilder writer instance.
>
> My suggestion was to not do it this way; are you agreeing with that,
> or saying something else?
>