You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Devanshu Vyas <vy...@gmail.com> on 2017/04/03 09:51:10 UTC

Re: Inconsistent String Comparisons

Hello Devs,

I have completed working on this task OFBIZ-9254
<https://issues.apache.org/jira/browse/OFBIZ-9254> for JAVA files, GROOVY
files and uploaded the patches also. Now, I am working on the same for FTL
files and found that there are two patterns used in OFBiz for string
comparisons:
    a) <#if roleType.roleTypeId.equals("_NA_")> selected="selected"</#if>
    b) <#if showLocation == "Y">
I think it will be better to define a standard way to compare strings in
FTL also. This will help in cleaning & improving the code.

Let me know your thoughts.



Thanks & Regards,
Devanshu Vyas.

On Sat, Feb 18, 2017 at 3:47 PM, Michael Brohl <mi...@ecomify.de>
wrote:

> +1, thanks, Devanshu!
>
> Regards,
>
> Michael
>
> Am 09.01.17 um 10:22 schrieb Devanshu Vyas:
>
>> Hello Devs,
>>
>> I found an inconsistency in the code for string comparison
>> *statusId.equals("PRUN_COMPLETED")* whereas it should be written as
>> *"PRUN_COMPLETED".equals(statusId)*
>> cause the former can throw NullPointerException if the variable found to
>> be
>> NULL.
>>
>> This code pattern can be found at several places and if you all agree with
>> this I can provide a patch for correcting code.
>>
>> Let me know your thoughts.
>>
>> Thanks & Regards,
>> Devanshu Vyas.
>>
>>
>
>

Re: Inconsistent String Comparisons

Posted by Devanshu Vyas <vy...@gmail.com>.
Thanks Jacques. Even I was more reluctant to use the b) option.

Now I will start my work for Freemarker files with the b) option i.e. '=='
operator.

Thanks & Regards,
Devanshu Vyas.

On Fri, Apr 21, 2017 at 1:40 PM, Jacques Le Roux <
jacques.le.roux@les7arts.com> wrote:

> Hi Devanshu ,
>
> I like to keep things simple (KISS way), reading
> http://freemarker.org/docs/dgui_template_exp.html#dgui_templ
> ate_exp_comparison
> I suggest we always use b) version. I see no reasons why a) was used, even
> historically.
>
> There are 2968 occurences of b) vs 77 of a) in OFBiz
>
> Thanks
>
> Jacques
>
> Le 03/04/2017 à 11:51, Devanshu Vyas a écrit :
>
>> Hello Devs,
>>
>> I have completed working on this task OFBIZ-9254
>> <https://issues.apache.org/jira/browse/OFBIZ-9254> for JAVA files, GROOVY
>>
>> files and uploaded the patches also. Now, I am working on the same for FTL
>> files and found that there are two patterns used in OFBiz for string
>> comparisons:
>>      a) <#if roleType.roleTypeId.equals("_NA_")>
>> selected="selected"</#if>
>>      b) <#if showLocation == "Y">
>> I think it will be better to define a standard way to compare strings in
>> FTL also. This will help in cleaning & improving the code.
>>
>> Let me know your thoughts.
>>
>>
>>
>> Thanks & Regards,
>> Devanshu Vyas.
>>
>> On Sat, Feb 18, 2017 at 3:47 PM, Michael Brohl <mi...@ecomify.de>
>> wrote:
>>
>> +1, thanks, Devanshu!
>>>
>>> Regards,
>>>
>>> Michael
>>>
>>> Am 09.01.17 um 10:22 schrieb Devanshu Vyas:
>>>
>>> Hello Devs,
>>>>
>>>> I found an inconsistency in the code for string comparison
>>>> *statusId.equals("PRUN_COMPLETED")* whereas it should be written as
>>>> *"PRUN_COMPLETED".equals(statusId)*
>>>> cause the former can throw NullPointerException if the variable found to
>>>> be
>>>> NULL.
>>>>
>>>> This code pattern can be found at several places and if you all agree
>>>> with
>>>> this I can provide a patch for correcting code.
>>>>
>>>> Let me know your thoughts.
>>>>
>>>> Thanks & Regards,
>>>> Devanshu Vyas.
>>>>
>>>>
>>>>
>>>
>

Re: Inconsistent String Comparisons

Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Devanshu ,

I like to keep things simple (KISS way), reading
http://freemarker.org/docs/dgui_template_exp.html#dgui_template_exp_comparison
I suggest we always use b) version. I see no reasons why a) was used, even historically.

There are 2968 occurences of b) vs 77 of a) in OFBiz

Thanks

Jacques

Le 03/04/2017 � 11:51, Devanshu Vyas a �crit :
> Hello Devs,
>
> I have completed working on this task OFBIZ-9254
> <https://issues.apache.org/jira/browse/OFBIZ-9254> for JAVA files, GROOVY
> files and uploaded the patches also. Now, I am working on the same for FTL
> files and found that there are two patterns used in OFBiz for string
> comparisons:
>      a) <#if roleType.roleTypeId.equals("_NA_")> selected="selected"</#if>
>      b) <#if showLocation == "Y">
> I think it will be better to define a standard way to compare strings in
> FTL also. This will help in cleaning & improving the code.
>
> Let me know your thoughts.
>
>
>
> Thanks & Regards,
> Devanshu Vyas.
>
> On Sat, Feb 18, 2017 at 3:47 PM, Michael Brohl <mi...@ecomify.de>
> wrote:
>
>> +1, thanks, Devanshu!
>>
>> Regards,
>>
>> Michael
>>
>> Am 09.01.17 um 10:22 schrieb Devanshu Vyas:
>>
>>> Hello Devs,
>>>
>>> I found an inconsistency in the code for string comparison
>>> *statusId.equals("PRUN_COMPLETED")* whereas it should be written as
>>> *"PRUN_COMPLETED".equals(statusId)*
>>> cause the former can throw NullPointerException if the variable found to
>>> be
>>> NULL.
>>>
>>> This code pattern can be found at several places and if you all agree with
>>> this I can provide a patch for correcting code.
>>>
>>> Let me know your thoughts.
>>>
>>> Thanks & Regards,
>>> Devanshu Vyas.
>>>
>>>
>>