You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacques Le Roux <ja...@les7arts.com> on 2014/11/21 14:08:35 UTC
Re: svn commit: r1639927 [2/3] - in
/ofbiz/trunk/framework/widget/src/org/ofbiz/widget: artifact/ fo/ form/
html/ text/ xml/
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, adrianc@apache.org 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
Re: svn commit: r1639927 [2/3] - in
/ofbiz/trunk/framework/widget/src/org/ofbiz/widget: artifact/ fo/ form/
html/ text/ xml/
Posted by Jacques Le Roux <ja...@les7arts.com>.
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, adrianc@apache.org 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
>>>
>
Re: svn commit: r1639927 [2/3] - in /ofbiz/trunk/framework/widget/src/org/ofbiz/widget:
artifact/ fo/ form/ html/ text/ xml/
Posted by Adrian Crum <ad...@sandglass-software.com>.
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, adrianc@apache.org 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
>>
Re: svn commit: r1639927 [2/3] - in
/ofbiz/trunk/framework/widget/src/org/ofbiz/widget: artifact/ fo/ form/
html/ text/ xml/
Posted by Jacques Le Roux <ja...@les7arts.com>.
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, adrianc@apache.org 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
>