Inconsistent String Comparisons

[Devanshu Vyas-2]

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.

[Nicolas Malin-2]

Hello Devanshu,

Yes your welcome to submit a patch :)

Nicolas

Le 09/01/2017 à 10:22, Devanshu Vyas a écrit :

> 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.
>

[Michael Brohl-3]

+1

Regards,

Michael

Am 09.01.17 um 10:27 schrieb Nicolas Malin:

> Hello Devanshu,
>
> Yes your welcome to submit a patch :)
>
> Nicolas
>
> Le 09/01/2017 à 10:22, Devanshu Vyas a écrit :
>> 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.
>>
>

[Swapnil Mane]

+1 Devanshu.


- Best Regards,
Swapnil M Mane

On Mon, Jan 9, 2017 at 2:52 PM, Devanshu Vyas <[hidden email]>
wrote:

> 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.
>

[Pranay Pandey-3]

+1 Devanshu.

Best regards,

Pranay Pandey
HotWax Systems
http://www.hotwaxsystems.com/

On Mon, Jan 9, 2017 at 2:52 PM, Devanshu Vyas <[hidden email]>
wrote:

> 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.
>

[Ratnesh Upadhyay-2]

+1 for the initiation, also we can enforce it as best practice for future
implementations and patches.

Thanks!!

Regards,
Ratnesh Upadhyay
HotWax Systems
www.hotwaxsystems.com

On Mon, Jan 9, 2017 at 2:52 PM, Devanshu Vyas <[hidden email]>
wrote:

> 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.
>

--
--
----------------------------------------------------
Cheers,
Thanks and Regards,
Ratnesh Upadhyay
Senior Business Analyst (Enterprise Software Solutions)
HotWax Media, Inc.
http://www.hotwaxmedia.com
----------------------------------------------------
E-mail : [hidden email]
Mobile : [phone]

[taher]

+1

On Feb 18, 2017 8:16 AM, "Ratnesh Upadhyay" <[hidden email]>
wrote:

+1 for the initiation, also we can enforce it as best practice for future
implementations and patches.

Thanks!!

Regards,
Ratnesh Upadhyay
HotWax Systems
www.hotwaxsystems.com

On Mon, Jan 9, 2017 at 2:52 PM, Devanshu Vyas <[hidden email]>
wrote:

> 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.
>

--
--
----------------------------------------------------
Cheers,
Thanks and Regards,
Ratnesh Upadhyay
Senior Business Analyst (Enterprise Software Solutions)
HotWax Media, Inc.
http://www.hotwaxmedia.com
----------------------------------------------------
E-mail : [hidden email]
Mobile : [phone]

[Jacques Le Roux]

+1, I suggest a Jira to handle all cases like that

Jacques


Le 09/01/2017 à 10:22, Devanshu Vyas a écrit :

> 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.
>

[Michael Brohl-3]

+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.
>

[Devanshu Vyas-2]

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 <[hidden email]>
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.
>>
>>
>
>

[Jacques Le Roux]

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 <[hidden email]>
> 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.
>>>
>>>
>>

[Devanshu Vyas-2]

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 <
[hidden email]> 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 <[hidden email]>
>> 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.
>>>>
>>>>
>>>>
>>>
>