You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Adam Heath <do...@brainfood.com> on 2010/02/02 21:05:20 UTC

Re: FlexibleStringExpander

Adrian Crum wrote:
> FlexibleStringExpander fse =
> FlexibleStringExpander.getInstance("someExpression");
> if (fse != null) {
>   // some code
> }

This was always broken.  Type foo = new Type();  foo will *never* be
null, period.  There are existing places in the code where this new
Type() compared to null takes place, unrelated to FSE.

Re: FlexibleStringExpander

Posted by Adrian Crum <ad...@hlmksw.com>.
Adam Heath wrote:
> Adrian Crum wrote:
>> Adam Heath wrote:
>>> Adrian Crum wrote:
>>>> FlexibleStringExpander fse =
>>>> FlexibleStringExpander.getInstance("someExpression");
>>>> if (fse != null) {
>>>>   // some code
>>>> }
>>> This was always broken.  Type foo = new Type();  foo will *never* be
>>> null, period.  There are existing places in the code where this new
>>> Type() compared to null takes place, unrelated to FSE.
>> I was trying to provide a simplified example. You are correct - the
>> simplified example would never work to begin with. What the simplified
>> example might really look like:
>>
>> protected FlexibleStringExpander description = null;
> 
> Just for reference, the correct fix here would remove the null default
> value, make it final, and remove the condition check below.

Exactly.


Re: FlexibleStringExpander

Posted by Adam Heath <do...@brainfood.com>.
Adrian Crum wrote:
> Adam Heath wrote:
>> Adrian Crum wrote:
>>> FlexibleStringExpander fse =
>>> FlexibleStringExpander.getInstance("someExpression");
>>> if (fse != null) {
>>>   // some code
>>> }
>>
>> This was always broken.  Type foo = new Type();  foo will *never* be
>> null, period.  There are existing places in the code where this new
>> Type() compared to null takes place, unrelated to FSE.
> 
> I was trying to provide a simplified example. You are correct - the
> simplified example would never work to begin with. What the simplified
> example might really look like:
> 
> protected FlexibleStringExpander description = null;

Just for reference, the correct fix here would remove the null default
value, make it final, and remove the condition check below.


> ...
> public MyClass(String expression) {
>   if (expression != null) {
>     description = FlexibleStringExpander.getInstance(expression);
>   }
> }
> ...
> public String getDescription(Map context) {
>   if (description != null) {
>     return description.expandString(context);
>   }
>   ...
> }
> 
> -Adrian
> 


Re: FlexibleStringExpander

Posted by Adrian Crum <ad...@hlmksw.com>.
Adam Heath wrote:
> Adrian Crum wrote:
>> FlexibleStringExpander fse =
>> FlexibleStringExpander.getInstance("someExpression");
>> if (fse != null) {
>>   // some code
>> }
> 
> This was always broken.  Type foo = new Type();  foo will *never* be
> null, period.  There are existing places in the code where this new
> Type() compared to null takes place, unrelated to FSE.

I was trying to provide a simplified example. You are correct - the 
simplified example would never work to begin with. What the simplified 
example might really look like:

protected FlexibleStringExpander description = null;
...
public MyClass(String expression) {
   if (expression != null) {
     description = FlexibleStringExpander.getInstance(expression);
   }
}
...
public String getDescription(Map context) {
   if (description != null) {
     return description.expandString(context);
   }
   ...
}

-Adrian