Re: svn commit: r1345473 - /ofbiz/trunk/specialpurpose/ecommerce/webapp/ecommerce/catalog/productdetail.ftl

[Jacopo Cappellato-4]

On Jun 2, 2012, at 11:46 AM, [hidden email] wrote:

> -        <#if productImageList != null && productImageList?has_content>
> +        <#if productImageList?has_content && productImageList?has_content>

Jacques, please double check the change you did.
As a side note, it would be better to revert this and review and commit:

https://issues.apache.org/jira/browse/OFBIZ-4916

Jacopo

[Jacques Le Roux]

Jacopo,

I vaguely remembered OFBIZ-4916, but as has_content is not deprecated
http://freemarker.sourceforge.net/docs/ref_depr_builtin.html
http://freemarker.sourceforge.net/docs/ref_builtins_expert.html#ref_builtin_has_content

I did not see any reasons to not use it there. I see it as UtilValidate.isNotEmtpy() in Java. I like the idea of having null and
empty checked at the same time (belt and suspenders). And last but not least I find it easier to read than ?? which is not exactly
the same. It's the deprecated ?exists, I proposed to use long ago http://markmail.org/message/6ipezxj65gcwlann

So since has_content tests for null value I don't see the need to repeat with something like

<#if productImageList?? && productImageList?has_content>
as has_content implies/includes ??

Did I miss something? I wonder now why in the past there was a test on null...

Jacques
PS: oops, I'm really tired, I just realise that I have duplicated the check :D, OK I fix. Or rather you are right I will revert and
review commit OFBIZ-4916. Thanks for the link!

From: "Jacopo Cappellato" <[hidden email]>

>
> On Jun 2, 2012, at 11:46 AM, [hidden email] wrote:
>
>> -        <#if productImageList != null && productImageList?has_content>
>> +        <#if productImageList?has_content && productImageList?has_content>
>
> Jacques, please double check the change you did.
> As a side note, it would be better to revert this and review and commit:
>
> https://issues.apache.org/jira/browse/OFBIZ-4916
>
> Jacopo
>

[Jacopo Cappellato-4]

On Jun 2, 2012, at 12:43 PM, Jacques Le Roux wrote:

> PS: oops, I'm really tired, I just realise that I have duplicated the check

Yes, this is what I meant

Jacopo

[Jacques Le Roux]

BTW I find the definition of ?? unclear at
http://freemarker.sourceforge.net/docs/dgui_template_exp.html#dgui_template_exp_missing_test

When the one of has_content  is much more clear and complete
http://freemarker.sourceforge.net/docs/ref_builtins_expert.html#ref_builtin_has_content

Jacques

From: "Jacques Le Roux" <[hidden email]>

> Jacopo,
>
> I vaguely remembered OFBIZ-4916, but as has_content is not deprecated
> http://freemarker.sourceforge.net/docs/ref_depr_builtin.html
> http://freemarker.sourceforge.net/docs/ref_builtins_expert.html#ref_builtin_has_content
>
> I did not see any reasons to not use it there. I see it as UtilValidate.isNotEmtpy() in Java. I like the idea of having null and
> empty checked at the same time (belt and suspenders). And last but not least I find it easier to read than ?? which is not exactly
> the same. It's the deprecated ?exists, I proposed to use long ago http://markmail.org/message/6ipezxj65gcwlann
>
> So since has_content tests for null value I don't see the need to repeat with something like
>
> <#if productImageList?? && productImageList?has_content>
> as has_content implies/includes ??
>
> Did I miss something? I wonder now why in the past there was a test on null...
>
> Jacques
> PS: oops, I'm really tired, I just realise that I have duplicated the check :D, OK I fix. Or rather you are right I will revert
> and
> review commit OFBIZ-4916. Thanks for the link!
>
> From: "Jacopo Cappellato" <[hidden email]>
>>
>> On Jun 2, 2012, at 11:46 AM, [hidden email] wrote:
>>
>>> -        <#if productImageList != null && productImageList?has_content>
>>> +        <#if productImageList?has_content && productImageList?has_content>
>>
>> Jacques, please double check the change you did.
>> As a side note, it would be better to revert this and review and commit:
>>
>> https://issues.apache.org/jira/browse/OFBIZ-4916
>>
>> Jacopo
>>
>
>

[Jacques Le Roux]

I guess ?? maps to != null, while "I'm sure" has_content maps to UtilValidate.isNotEmtpy()

I'ts easier to follow a flow in Java to know if a value can be null or not (than in Freemarker). This mostly thanks to the modern
IDEs
That's why I prefer to use use has_content. Even if it's temptimp to use ?? instead when you are sure you only check for a NPE


Jacques

From: "Jacques Le Roux" <[hidden email]>

> BTW I find the definition of ?? unclear at
> http://freemarker.sourceforge.net/docs/dgui_template_exp.html#dgui_template_exp_missing_test
>
> When the one of has_content  is much more clear and complete
> http://freemarker.sourceforge.net/docs/ref_builtins_expert.html#ref_builtin_has_content
>
> Jacques
>
> From: "Jacques Le Roux" <[hidden email]>
>> Jacopo,
>>
>> I vaguely remembered OFBIZ-4916, but as has_content is not deprecated
>> http://freemarker.sourceforge.net/docs/ref_depr_builtin.html
>> http://freemarker.sourceforge.net/docs/ref_builtins_expert.html#ref_builtin_has_content
>>
>> I did not see any reasons to not use it there. I see it as UtilValidate.isNotEmtpy() in Java. I like the idea of having null and
>> empty checked at the same time (belt and suspenders). And last but not least I find it easier to read than ?? which is not
>> exactly
>> the same. It's the deprecated ?exists, I proposed to use long ago http://markmail.org/message/6ipezxj65gcwlann
>>
>> So since has_content tests for null value I don't see the need to repeat with something like
>>
>> <#if productImageList?? && productImageList?has_content>
>> as has_content implies/includes ??
>>
>> Did I miss something? I wonder now why in the past there was a test on null...
>>
>> Jacques
>> PS: oops, I'm really tired, I just realise that I have duplicated the check :D, OK I fix. Or rather you are right I will revert
>> and
>> review commit OFBIZ-4916. Thanks for the link!
>>
>> From: "Jacopo Cappellato" <[hidden email]>
>>>
>>> On Jun 2, 2012, at 11:46 AM, [hidden email] wrote:
>>>
>>>> -        <#if productImageList != null && productImageList?has_content>
>>>> +        <#if productImageList?has_content && productImageList?has_content>
>>>
>>> Jacques, please double check the change you did.
>>> As a side note, it would be better to revert this and review and commit:
>>>
>>> https://issues.apache.org/jira/browse/OFBIZ-4916
>>>
>>> Jacopo
>>>
>>
>>
>