OFBiz › OFBiz - Dev

Re: svn commit: r1639927 [2/3] - in /ofbiz/trunk/framework/widget/src/org/ofbiz/widget: artifact/ fo/ form/ html/ text/ xml/

‹ Previous Topic Next Topic ›
 
classic Classic list List threaded Threaded
♦
♦
 4 messages Options
Jacques Le Roux
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: svn commit: r1639927 [2/3] - in /ofbiz/trunk/framework/widget/src/org/ofbiz/widget: artifact/ fo/ form/ html/ text/ xml/

Jacques Le Roux
Administrator
Hi Adrian,

I don't want to be nit picky, but I personnally prefer the 1st version (but the useless void after "} else if (" on 1st line)

Le 15/11/2014 22:33, [hidden email] a écrit :

> -            } else if (
> -                "value".equals(modelField.getType())
> -                    || "comment".equals(modelField.getType())
> -                    || "description".equals(modelField.getType())
> -                    || "long-varchar".equals(modelField.getType())
> -                    || "url".equals(modelField.getType())
> -                    || "email".equals(modelField.getType())) {
> -                ModelFormField.TextFindField textField = new ModelFormField.TextFindField(ModelFormField.FieldInfo.SOURCE_AUTO_ENTITY, this);
> +            } else if ("value".equals(modelField.getType()) || "comment".equals(modelField.getType())
> +                    || "description".equals(modelField.getType()) || "long-varchar".equals(modelField.getType())
> +                    || "url".equals(modelField.getType()) || "email".equals(modelField.getType())) {
> +                ModelFormField.TextFindField textField = new ModelFormField.TextFindField(FieldInfo.SOURCE_AUTO_ENTITY, this);
>                   textField.setSize(60);
>                   textField.setMaxlength(Integer.valueOf(250));

I find it easier to read. I saw few blocks like this.

Jacques
Jacques Le Roux
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: svn commit: r1639927 [2/3] - in /ofbiz/trunk/framework/widget/src/org/ofbiz/widget: artifact/ fo/ form/ html/ text/ xml/

Jacques Le Roux
Administrator
Also in the same commit

-            if ("date".equals(this.type))  return (new java.sql.Date(System.currentTimeMillis())).toString();
-            else if ("time".equals(this.type)) return (new java.sql.Time(System.currentTimeMillis())).toString();
-            else return UtilDateTime.nowTimestamp().toString();
+            if ("date".equals(this.type))
+                return (new java.sql.Date(System.currentTimeMillis())).toString();
+            else if ("time".equals(this.type))
+                return (new java.sql.Time(System.currentTimeMillis())).toString();
+            else
+                return UtilDateTime.nowTimestamp().toString();

We already discussed on this and I'd like to have a consensus

Personally I prefer the 1st form over the 2nd because it's less error prone. It's obvious  you can't add a line. I know it's here returns, but it just
an example there are much other cases like that (not returns) in this commit.

Of course best is the 2nd form when { } are used.

This said this was an huge overhaul, and I can understand why you skipped the { }

Jacques

Le 21/11/2014 14:08, Jacques Le Roux a écrit :

> Hi Adrian,
>
> I don't want to be nit picky, but I personnally prefer the 1st version (but the useless void after "} else if (" on 1st line)
>
> Le 15/11/2014 22:33, [hidden email] a écrit :
>> -            } else if (
>> -                "value".equals(modelField.getType())
>> -                    || "comment".equals(modelField.getType())
>> -                    || "description".equals(modelField.getType())
>> -                    || "long-varchar".equals(modelField.getType())
>> -                    || "url".equals(modelField.getType())
>> -                    || "email".equals(modelField.getType())) {
>> -                ModelFormField.TextFindField textField = new ModelFormField.TextFindField(ModelFormField.FieldInfo.SOURCE_AUTO_ENTITY, this);
>> +            } else if ("value".equals(modelField.getType()) || "comment".equals(modelField.getType())
>> +                    || "description".equals(modelField.getType()) || "long-varchar".equals(modelField.getType())
>> +                    || "url".equals(modelField.getType()) || "email".equals(modelField.getType())) {
>> +                ModelFormField.TextFindField textField = new ModelFormField.TextFindField(FieldInfo.SOURCE_AUTO_ENTITY, this);
>>                   textField.setSize(60);
>>                   textField.setMaxlength(Integer.valueOf(250));
>
> I find it easier to read. I saw few blocks like this.
>
> Jacques
>
Adrian Crum-3
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: svn commit: r1639927 [2/3] - in /ofbiz/trunk/framework/widget/src/org/ofbiz/widget: artifact/ fo/ form/ html/ text/ xml/

Adrian Crum-3
The formatting was an accident. I selected a block of text I inserted
and told Eclipse to format it, but Eclipse formatted the entire file and
not just my selection. I didn't notice the whole file was formatted
until after my commit.

I will try to put the formatting back to the way it was.

Adrian Crum
Sandglass Software
www.sandglass-software.com

On 11/21/2014 2:56 PM, Jacques Le Roux wrote:

> Also in the same commit
>
> -            if ("date".equals(this.type))  return (new
> java.sql.Date(System.currentTimeMillis())).toString();
> -            else if ("time".equals(this.type)) return (new
> java.sql.Time(System.currentTimeMillis())).toString();
> -            else return UtilDateTime.nowTimestamp().toString();
> +            if ("date".equals(this.type))
> +                return (new
> java.sql.Date(System.currentTimeMillis())).toString();
> +            else if ("time".equals(this.type))
> +                return (new
> java.sql.Time(System.currentTimeMillis())).toString();
> +            else
> +                return UtilDateTime.nowTimestamp().toString();
>
> We already discussed on this and I'd like to have a consensus
>
> Personally I prefer the 1st form over the 2nd because it's less error
> prone. It's obvious  you can't add a line. I know it's here returns, but
> it just an example there are much other cases like that (not returns) in
> this commit.
>
> Of course best is the 2nd form when { } are used.
>
> This said this was an huge overhaul, and I can understand why you
> skipped the { }
>
> Jacques
>
> Le 21/11/2014 14:08, Jacques Le Roux a écrit :
>> Hi Adrian,
>>
>> I don't want to be nit picky, but I personnally prefer the 1st version
>> (but the useless void after "} else if (" on 1st line)
>>
>> Le 15/11/2014 22:33, [hidden email] a écrit :
>>> -            } else if (
>>> -                "value".equals(modelField.getType())
>>> -                    || "comment".equals(modelField.getType())
>>> -                    || "description".equals(modelField.getType())
>>> -                    || "long-varchar".equals(modelField.getType())
>>> -                    || "url".equals(modelField.getType())
>>> -                    || "email".equals(modelField.getType())) {
>>> -                ModelFormField.TextFindField textField = new
>>> ModelFormField.TextFindField(ModelFormField.FieldInfo.SOURCE_AUTO_ENTITY,
>>> this);
>>> +            } else if ("value".equals(modelField.getType()) ||
>>> "comment".equals(modelField.getType())
>>> +                    || "description".equals(modelField.getType()) ||
>>> "long-varchar".equals(modelField.getType())
>>> +                    || "url".equals(modelField.getType()) ||
>>> "email".equals(modelField.getType())) {
>>> +                ModelFormField.TextFindField textField = new
>>> ModelFormField.TextFindField(FieldInfo.SOURCE_AUTO_ENTITY, this);
>>>                   textField.setSize(60);
>>>                   textField.setMaxlength(Integer.valueOf(250));
>>
>> I find it easier to read. I saw few blocks like this.
>>
>> Jacques
>>
Jacques Le Roux
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: svn commit: r1639927 [2/3] - in /ofbiz/trunk/framework/widget/src/org/ofbiz/widget: artifact/ fo/ form/ html/ text/ xml/

Jacques Le Roux
Administrator
Yes it happened to me too

Thanks and sorry for the extra work

Jacques

Le 21/11/2014 16:13, Adrian Crum a écrit :

> The formatting was an accident. I selected a block of text I inserted and told Eclipse to format it, but Eclipse formatted the entire file and not
> just my selection. I didn't notice the whole file was formatted until after my commit.
>
> I will try to put the formatting back to the way it was.
>
> Adrian Crum
> Sandglass Software
> www.sandglass-software.com
>
> On 11/21/2014 2:56 PM, Jacques Le Roux wrote:
>> Also in the same commit
>>
>> -            if ("date".equals(this.type))  return (new
>> java.sql.Date(System.currentTimeMillis())).toString();
>> -            else if ("time".equals(this.type)) return (new
>> java.sql.Time(System.currentTimeMillis())).toString();
>> -            else return UtilDateTime.nowTimestamp().toString();
>> +            if ("date".equals(this.type))
>> +                return (new
>> java.sql.Date(System.currentTimeMillis())).toString();
>> +            else if ("time".equals(this.type))
>> +                return (new
>> java.sql.Time(System.currentTimeMillis())).toString();
>> +            else
>> +                return UtilDateTime.nowTimestamp().toString();
>>
>> We already discussed on this and I'd like to have a consensus
>>
>> Personally I prefer the 1st form over the 2nd because it's less error
>> prone. It's obvious  you can't add a line. I know it's here returns, but
>> it just an example there are much other cases like that (not returns) in
>> this commit.
>>
>> Of course best is the 2nd form when { } are used.
>>
>> This said this was an huge overhaul, and I can understand why you
>> skipped the { }
>>
>> Jacques
>>
>> Le 21/11/2014 14:08, Jacques Le Roux a écrit :
>>> Hi Adrian,
>>>
>>> I don't want to be nit picky, but I personnally prefer the 1st version
>>> (but the useless void after "} else if (" on 1st line)
>>>
>>> Le 15/11/2014 22:33, [hidden email] a écrit :
>>>> -            } else if (
>>>> -                "value".equals(modelField.getType())
>>>> -                    || "comment".equals(modelField.getType())
>>>> -                    || "description".equals(modelField.getType())
>>>> -                    || "long-varchar".equals(modelField.getType())
>>>> -                    || "url".equals(modelField.getType())
>>>> -                    || "email".equals(modelField.getType())) {
>>>> -                ModelFormField.TextFindField textField = new
>>>> ModelFormField.TextFindField(ModelFormField.FieldInfo.SOURCE_AUTO_ENTITY,
>>>> this);
>>>> +            } else if ("value".equals(modelField.getType()) ||
>>>> "comment".equals(modelField.getType())
>>>> +                    || "description".equals(modelField.getType()) ||
>>>> "long-varchar".equals(modelField.getType())
>>>> +                    || "url".equals(modelField.getType()) ||
>>>> "email".equals(modelField.getType())) {
>>>> +                ModelFormField.TextFindField textField = new
>>>> ModelFormField.TextFindField(FieldInfo.SOURCE_AUTO_ENTITY, this);
>>>>                   textField.setSize(60);
>>>> textField.setMaxlength(Integer.valueOf(250));
>>>
>>> I find it easier to read. I saw few blocks like this.
>>>
>>> Jacques
>>>
>
Free forum by Nabble Edit this page