Re: svn commit: r1813637 - in /ofbiz/ofbiz-framework/trunk/framework/datafile/src/main/java/org/apache/of biz/datafile: DataFile.java DataFile2EntityXml.java ModelDataFileReader.java Record.java RecordIterator.java

[Jacques Le Roux]

Le 28/10/2017 à 16:45, [hidden email] a écrit :
> -    protected DataFile() {}
> +    protected DataFile() {
> +    }
Hi Michael,

Do we really prefer "this to that" ?

Jacques

[Jacques Le Roux]

Le 28/10/2017 à 16:45, [hidden email] a écrit :

>       /**
>        * little endian reader for 4 byte int.
>        */
> -    public final int readLEInt(byte []byteArray) {
> -    return
> -    (byteArray[3])      << 24 |
> -    (byteArray[2]&0xff) << 16 |
> -    (byteArray[1]&0xff) <<  8 |
> -    (byteArray[0]&0xff);
> +    public final int readLEInt(byte[] byteArray) {
> +        return (byteArray[3]) << 24 | (byteArray[2] & 0xff) << 16 | (byteArray[1] & 0xff) << 8 | (byteArray[0] & 0xff);
>       }
>  
>       /**
>        * little endian reader for 8 byte long.
>        */
> -    public final long readLELong(byte []byteArray) {
> -    return
> -    (long)(byteArray[7])      << 56 |  /* long cast needed or shift done modulo 32 */
> -    (long)(byteArray[6]&0xff) << 48 |
> -    (long)(byteArray[5]&0xff) << 40 |
> -    (long)(byteArray[4]&0xff) << 32 |
> -    (long)(byteArray[3]&0xff) << 24 |
> -    (long)(byteArray[2]&0xff) << 16 |
> -    (long)(byteArray[1]&0xff) <<  8 |
> -    (byteArray[0]&0xff);
> +    public final long readLELong(byte[] byteArray) {
> +        return (long) (byteArray[7]) << 56 | /* long cast needed or shift done modulo 32 */
> +               (long) (byteArray[6] & 0xff) << 48 | (long) (byteArray[5] & 0xff) << 40 | (long) (byteArray[4] & 0xff) << 32 | (long) (byteArray[3] & 0xff) << 24
> +               | (long) (byteArray[2] & 0xff) << 16 | (long) (byteArray[1] & 0xff) << 8 | (byteArray[0] & 0xff);
>       }

I agree with your formatting Michael,

But is everybody happy with it, especially the 2n one?

Jacques

[Jacques Le Roux]

Le 28/10/2017 à 16:45, [hidden email] a écrit :
> -        for (int i = 0; i < modelField.length; i++)
> -            sb.append(PAD_CHAR);
> -        data = sb.toString();
> -        }
> +                for (int i = 0; i < modelField.length; i++)
> +                    sb.append(PAD_CHAR);
> +                data = sb.toString();
> +            }
>  
Hi All,

I find this ambiguous. We have few options here:

1)
                 for (int i = 0; i < modelField.length; i++)
                     sb.append(PAD_CHAR);

                 data = sb.toString();

2)
                 for (int i = 0; i < modelField.length; i++) {
                     sb.append(PAD_CHAR);
                 }
                 data = sb.toString();

3)
                 for (int i = 0; i < modelField.length; i++) sb.append(PAD_CHAR);
                 data = sb.toString();

4)
                 for (int i = 0; i < modelField.length; i++) sb.append(PAD_CHAR);

                 data = sb.toString();

Which one do you prefer? Of course this should be a general rule, not only for this case!

Thanks

Jacques

[Michael Brohl-3]

I think this is just a formatting change becaused I used our formatter
for Eclipse.

Not really something worth a discussion, is it?


Am 29.10.17 um 10:07 schrieb Jacques Le Roux:

> Le 28/10/2017 à 16:45, [hidden email] a écrit :
>> -        for (int i = 0; i < modelField.length; i++)
>> -            sb.append(PAD_CHAR);
>> -        data = sb.toString();
>> -        }
>> +                for (int i = 0; i < modelField.length; i++)
>> +                    sb.append(PAD_CHAR);
>> +                data = sb.toString();
>> +            }
> Hi All,
>
> I find this ambiguous. We have few options here:
>
> 1)
>                 for (int i = 0; i < modelField.length; i++)
>                     sb.append(PAD_CHAR);
>
>                 data = sb.toString();
>
> 2)
>                 for (int i = 0; i < modelField.length; i++) {
>                     sb.append(PAD_CHAR);
>                 }
>                 data = sb.toString();
>
> 3)
>                 for (int i = 0; i < modelField.length; i++)
> sb.append(PAD_CHAR);
>                 data = sb.toString();
>
> 4)
>                 for (int i = 0; i < modelField.length; i++)
> sb.append(PAD_CHAR);
>
>                 data = sb.toString();
>
> Which one do you prefer? Of course this should be a general rule, not
> only for this case!
>
> Thanks
>
> Jacques
>

[Michael Brohl-3]

Same thing as before...

Am 29.10.17 um 09:56 schrieb Jacques Le Roux:

> Le 28/10/2017 à 16:45, [hidden email] a écrit :
>>       /**
>>        * little endian reader for 4 byte int.
>>        */
>> -    public final int readLEInt(byte []byteArray) {
>> -    return
>> -    (byteArray[3])      << 24 |
>> -    (byteArray[2]&0xff) << 16 |
>> -    (byteArray[1]&0xff) <<  8 |
>> -    (byteArray[0]&0xff);
>> +    public final int readLEInt(byte[] byteArray) {
>> +        return (byteArray[3]) << 24 | (byteArray[2] & 0xff) << 16 |
>> (byteArray[1] & 0xff) << 8 | (byteArray[0] & 0xff);
>>       }
>>         /**
>>        * little endian reader for 8 byte long.
>>        */
>> -    public final long readLELong(byte []byteArray) {
>> -    return
>> -    (long)(byteArray[7])      << 56 |  /* long cast needed or shift
>> done modulo 32 */
>> -    (long)(byteArray[6]&0xff) << 48 |
>> -    (long)(byteArray[5]&0xff) << 40 |
>> -    (long)(byteArray[4]&0xff) << 32 |
>> -    (long)(byteArray[3]&0xff) << 24 |
>> -    (long)(byteArray[2]&0xff) << 16 |
>> -    (long)(byteArray[1]&0xff) <<  8 |
>> -    (byteArray[0]&0xff);
>> +    public final long readLELong(byte[] byteArray) {
>> +        return (long) (byteArray[7]) << 56 | /* long cast needed or
>> shift done modulo 32 */
>> +               (long) (byteArray[6] & 0xff) << 48 | (long)
>> (byteArray[5] & 0xff) << 40 | (long) (byteArray[4] & 0xff) << 32 |
>> (long) (byteArray[3] & 0xff) << 24
>> +               | (long) (byteArray[2] & 0xff) << 16 | (long)
>> (byteArray[1] & 0xff) << 8 | (byteArray[0] & 0xff);
>>       }
> I agree with your formatting Michael,
>
> But is everybody happy with it, especially the 2n one?
>
> Jacques
>

[Michael Brohl-3]

Same thing as before.

This is just produced by the formatter.


Am 29.10.17 um 09:15 schrieb Jacques Le Roux:

> Le 28/10/2017 à 16:45, [hidden email] a écrit :
>> -    protected DataFile() {}
>> +    protected DataFile() {
>> +    }
> Hi Michael,
>
> Do we really prefer "this to that" ?
>
> Jacques
>

[Jacques Le Roux]

We (committers at least, with contributors would even be better) could decide to use the same formatter.

This would helps when merging from custom projects or backporting to them...

I tried this once by sharing my Eclipse formatter at https://cwiki.apache.org/confluence/pages/viewpageattachments.action?pageId=7766027

So we could decide on our own rules and get more legible and consistent formatting

Now that more people use InteliJ we could start from an Eclipse formatter

https://blog.jetbrains.com/idea/2014/01/intellij-idea-13-importing-code-formatter-settings-from-eclipse/
https://plugins.jetbrains.com/plugin/6546-eclipse-code-formatter

It seems there is no other way around

https://stackoverflow.com/questions/29226589/export-intellij-idea-code-formatting-rules-to-eclipse

Maybe ultimately using CheckStyle as suggested? I remember being rebuffed a decade abo when I suggested to use Findbugs...

I'm not strongly opinionated about that, but when I think about external mergings...

Jacques


Le 29/10/2017 à 10:53, Michael Brohl a écrit :

> I think this is just a formatting change becaused I used our formatter for Eclipse.
>
> Not really something worth a discussion, is it?
>
>
> Am 29.10.17 um 10:07 schrieb Jacques Le Roux:
>> Le 28/10/2017 à 16:45, [hidden email] a écrit :
>>> -        for (int i = 0; i < modelField.length; i++)
>>> -            sb.append(PAD_CHAR);
>>> -        data = sb.toString();
>>> -        }
>>> +                for (int i = 0; i < modelField.length; i++)
>>> +                    sb.append(PAD_CHAR);
>>> +                data = sb.toString();
>>> +            }
>> Hi All,
>>
>> I find this ambiguous. We have few options here:
>>
>> 1)
>>                 for (int i = 0; i < modelField.length; i++)
>>                     sb.append(PAD_CHAR);
>>
>>                 data = sb.toString();
>>
>> 2)
>>                 for (int i = 0; i < modelField.length; i++) {
>>                     sb.append(PAD_CHAR);
>>                 }
>>                 data = sb.toString();
>>
>> 3)
>>                 for (int i = 0; i < modelField.length; i++) sb.append(PAD_CHAR);
>>                 data = sb.toString();
>>
>> 4)
>>                 for (int i = 0; i < modelField.length; i++) sb.append(PAD_CHAR);
>>
>>                 data = sb.toString();
>>
>> Which one do you prefer? Of course this should be a general rule, not only for this case!
>>
>> Thanks
>>
>> Jacques
>>
>

[taher]

Unless we find a way of automating, I don't think we should spend much time
on things like how to write an empty constructor. We have bigger problems
to solve.

On Oct 29, 2017 1:36 PM, "Jacques Le Roux" <[hidden email]>
wrote:

> We (committers at least, with contributors would even be better) could
> decide to use the same formatter.
>
> This would helps when merging from custom projects or backporting to
> them...
>
> I tried this once by sharing my Eclipse formatter at
> https://cwiki.apache.org/confluence/pages/viewpageattachment
> s.action?pageId=7766027
>
> So we could decide on our own rules and get more legible and consistent
> formatting
>
> Now that more people use InteliJ we could start from an Eclipse formatter
>
> https://blog.jetbrains.com/idea/2014/01/intellij-idea-13-imp
> orting-code-formatter-settings-from-eclipse/
> https://plugins.jetbrains.com/plugin/6546-eclipse-code-formatter
>
> It seems there is no other way around
>
> https://stackoverflow.com/questions/29226589/export-intellij
> -idea-code-formatting-rules-to-eclipse
>
> Maybe ultimately using CheckStyle as suggested? I remember being rebuffed
> a decade abo when I suggested to use Findbugs...
>
> I'm not strongly opinionated about that, but when I think about external
> mergings...
>
> Jacques
>
>
> Le 29/10/2017 à 10:53, Michael Brohl a écrit :
>
>> I think this is just a formatting change becaused I used our formatter
>> for Eclipse.
>>
>> Not really something worth a discussion, is it?
>>
>>
>> Am 29.10.17 um 10:07 schrieb Jacques Le Roux:
>>
>>> Le 28/10/2017 à 16:45, [hidden email] a écrit :
>>>
>>>> -        for (int i = 0; i < modelField.length; i++)
>>>> -            sb.append(PAD_CHAR);
>>>> -        data = sb.toString();
>>>> -        }
>>>> +                for (int i = 0; i < modelField.length; i++)
>>>> +                    sb.append(PAD_CHAR);
>>>> +                data = sb.toString();
>>>> +            }
>>>>
>>> Hi All,
>>>
>>> I find this ambiguous. We have few options here:
>>>
>>> 1)
>>>                 for (int i = 0; i < modelField.length; i++)
>>>                     sb.append(PAD_CHAR);
>>>
>>>                 data = sb.toString();
>>>
>>> 2)
>>>                 for (int i = 0; i < modelField.length; i++) {
>>>                     sb.append(PAD_CHAR);
>>>                 }
>>>                 data = sb.toString();
>>>
>>> 3)
>>>                 for (int i = 0; i < modelField.length; i++)
>>> sb.append(PAD_CHAR);
>>>                 data = sb.toString();
>>>
>>> 4)
>>>                 for (int i = 0; i < modelField.length; i++)
>>> sb.append(PAD_CHAR);
>>>
>>>                 data = sb.toString();
>>>
>>> Which one do you prefer? Of course this should be a general rule, not
>>> only for this case!
>>>
>>> Thanks
>>>
>>> Jacques
>>>
>>>
>>
>

[Jacques Le Roux]

Using a checkstyle Gradle task could help in this way, notably by running it in Buildbot.

But we need 1st to agree on rules...

Jacques


Le 29/10/2017 à 11:57, Taher Alkhateeb a écrit :

> Unless we find a way of automating, I don't think we should spend much time
> on things like how to write an empty constructor. We have bigger problems
> to solve.
>
> On Oct 29, 2017 1:36 PM, "Jacques Le Roux" <[hidden email]>
> wrote:
>
>> We (committers at least, with contributors would even be better) could
>> decide to use the same formatter.
>>
>> This would helps when merging from custom projects or backporting to
>> them...
>>
>> I tried this once by sharing my Eclipse formatter at
>> https://cwiki.apache.org/confluence/pages/viewpageattachment
>> s.action?pageId=7766027
>>
>> So we could decide on our own rules and get more legible and consistent
>> formatting
>>
>> Now that more people use InteliJ we could start from an Eclipse formatter
>>
>> https://blog.jetbrains.com/idea/2014/01/intellij-idea-13-imp
>> orting-code-formatter-settings-from-eclipse/
>> https://plugins.jetbrains.com/plugin/6546-eclipse-code-formatter
>>
>> It seems there is no other way around
>>
>> https://stackoverflow.com/questions/29226589/export-intellij
>> -idea-code-formatting-rules-to-eclipse
>>
>> Maybe ultimately using CheckStyle as suggested? I remember being rebuffed
>> a decade abo when I suggested to use Findbugs...
>>
>> I'm not strongly opinionated about that, but when I think about external
>> mergings...
>>
>> Jacques
>>
>>
>> Le 29/10/2017 à 10:53, Michael Brohl a écrit :
>>
>>> I think this is just a formatting change becaused I used our formatter
>>> for Eclipse.
>>>
>>> Not really something worth a discussion, is it?
>>>
>>>
>>> Am 29.10.17 um 10:07 schrieb Jacques Le Roux:
>>>
>>>> Le 28/10/2017 à 16:45, [hidden email] a écrit :
>>>>
>>>>> -        for (int i = 0; i < modelField.length; i++)
>>>>> -            sb.append(PAD_CHAR);
>>>>> -        data = sb.toString();
>>>>> -        }
>>>>> +                for (int i = 0; i < modelField.length; i++)
>>>>> +                    sb.append(PAD_CHAR);
>>>>> +                data = sb.toString();
>>>>> +            }
>>>>>
>>>> Hi All,
>>>>
>>>> I find this ambiguous. We have few options here:
>>>>
>>>> 1)
>>>>                  for (int i = 0; i < modelField.length; i++)
>>>>                      sb.append(PAD_CHAR);
>>>>
>>>>                  data = sb.toString();
>>>>
>>>> 2)
>>>>                  for (int i = 0; i < modelField.length; i++) {
>>>>                      sb.append(PAD_CHAR);
>>>>                  }
>>>>                  data = sb.toString();
>>>>
>>>> 3)
>>>>                  for (int i = 0; i < modelField.length; i++)
>>>> sb.append(PAD_CHAR);
>>>>                  data = sb.toString();
>>>>
>>>> 4)
>>>>                  for (int i = 0; i < modelField.length; i++)
>>>> sb.append(PAD_CHAR);
>>>>
>>>>                  data = sb.toString();
>>>>
>>>> Which one do you prefer? Of course this should be a general rule, not
>>>> only for this case!
>>>>
>>>> Thanks
>>>>
>>>> Jacques
>>>>
>>>>

[Michael Brohl-3]

Ah, I thought I have used the OFBiz formatter provided by the link you
mentioned, I have it embedded in Eclipse.

But was wrong, it was the Eclipse default formatter. I changed it back now.

I agree that we should all use the same formatting conventions to
prevent having "changes" which are only produced by formatting. The
attachment should be mentioned more prominently, as far as I can see it
is only depicted by the small icon just below the headline.

If we want to decide on a formatting style, best would be to have one
which is most near to what we have in the codebase now, if possible, to
avoid having too much changes in the future.

Regards,

Michael


Am 29.10.17 um 11:37 schrieb Jacques Le Roux:

> We (committers at least, with contributors would even be better) could
> decide to use the same formatter.
>
> This would helps when merging from custom projects or backporting to
> them...
>
> I tried this once by sharing my Eclipse formatter at
> https://cwiki.apache.org/confluence/pages/viewpageattachments.action?pageId=7766027
>
> So we could decide on our own rules and get more legible and
> consistent formatting
>
> Now that more people use InteliJ we could start from an Eclipse formatter
>
> https://blog.jetbrains.com/idea/2014/01/intellij-idea-13-importing-code-formatter-settings-from-eclipse/ 
>
> https://plugins.jetbrains.com/plugin/6546-eclipse-code-formatter
>
> It seems there is no other way around
>
> https://stackoverflow.com/questions/29226589/export-intellij-idea-code-formatting-rules-to-eclipse 
>
>
> Maybe ultimately using CheckStyle as suggested? I remember being
> rebuffed a decade abo when I suggested to use Findbugs...
>
> I'm not strongly opinionated about that, but when I think about
> external mergings...
>
> Jacques
>
>
> Le 29/10/2017 à 10:53, Michael Brohl a écrit :
>> I think this is just a formatting change becaused I used our
>> formatter for Eclipse.
>>
>> Not really something worth a discussion, is it?
>>
>>
>> Am 29.10.17 um 10:07 schrieb Jacques Le Roux:
>>> Le 28/10/2017 à 16:45, [hidden email] a écrit :
>>>> -        for (int i = 0; i < modelField.length; i++)
>>>> -            sb.append(PAD_CHAR);
>>>> -        data = sb.toString();
>>>> -        }
>>>> +                for (int i = 0; i < modelField.length; i++)
>>>> +                    sb.append(PAD_CHAR);
>>>> +                data = sb.toString();
>>>> +            }
>>> Hi All,
>>>
>>> I find this ambiguous. We have few options here:
>>>
>>> 1)
>>>                 for (int i = 0; i < modelField.length; i++)
>>>                     sb.append(PAD_CHAR);
>>>
>>>                 data = sb.toString();
>>>
>>> 2)
>>>                 for (int i = 0; i < modelField.length; i++) {
>>>                     sb.append(PAD_CHAR);
>>>                 }
>>>                 data = sb.toString();
>>>
>>> 3)
>>>                 for (int i = 0; i < modelField.length; i++)
>>> sb.append(PAD_CHAR);
>>>                 data = sb.toString();
>>>
>>> 4)
>>>                 for (int i = 0; i < modelField.length; i++)
>>> sb.append(PAD_CHAR);
>>>
>>>                 data = sb.toString();
>>>
>>> Which one do you prefer? Of course this should be a general rule,
>>> not only for this case!
>>>
>>> Thanks
>>>
>>> Jacques
>>>
>>
>