Re: svn commit: r925302 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java

[Adam Heath-2]

[hidden email] wrote:

> Author: erwan
> Date: Fri Mar 19 16:17:35 2010
> New Revision: 925302
>
> URL: http://svn.apache.org/viewvc?rev=925302&view=rev
> Log:
> Correcting warning message on too long foreign keys
>
> Modified:
>     ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java
>
> Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java?rev=925302&r1=925301&r2=925302&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java (original)
> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java Fri Mar 19 16:17:35 2010
> @@ -194,7 +194,8 @@ public class ModelEntityChecker {
>  
>                          // make sure all FK names are <= 18 characters
>                          if (relation.getFkName().length() > 18) {
> -                            warningList.add("[RelFKNameGT18] The foregn key name (length:" + relation.getFkName().length()
> +                            warningList.add("[RelFKNameGT18] The foreign key named " + relation.getFkName()
> +                                            + " (length:" + relation.getFkName().length()
>                                              + ") was greater than 18 characters in length for relation " + relation.getTitle() + relation.getRelEntityName()
>                                              + " of entity " + entity.getEntityName() + ".");
>                          }

Hmm.  What is the standard for overly long lines?  All on one, or
wrapped?  The code base is not consistent in this manner.

My opinion, is that when wrapping, it enforces a maximum size, that in
all likely hood is less than what we developers actually have
available.  This would then mean that we have tons of useless blank
space on the right side of our monitors, and that the code would end
up taking up more vertical lines.  Which is also bad, because then we
can't see the full method, when it gets large.

[Erwan de FERRIERES]

Le 19/03/2010 17:28, Adam Heath a écrit :

>
> Hmm.  What is the standard for overly long lines?  All on one, or
> wrapped?  The code base is not consistent in this manner.
>
> My opinion, is that when wrapping, it enforces a maximum size, that in
> all likely hood is less than what we developers actually have
> available.  This would then mean that we have tons of useless blank
> space on the right side of our monitors, and that the code would end
> up taking up more vertical lines.  Which is also bad, because then we
> can't see the full method, when it gets large.
>
>

Yes, we should define a right margin size, and then migrate, steps by
steps, all the code to this new coding style...

And what to do with xml and java files ?

--
Erwan de FERRIERES
www.nereide.biz

[Adam Heath-2]

Erwan de FERRIERES wrote:

> Le 19/03/2010 17:28, Adam Heath a écrit :
>
>>
>> Hmm.  What is the standard for overly long lines?  All on one, or
>> wrapped?  The code base is not consistent in this manner.
>>
>> My opinion, is that when wrapping, it enforces a maximum size, that in
>> all likely hood is less than what we developers actually have
>> available.  This would then mean that we have tons of useless blank
>> space on the right side of our monitors, and that the code would end
>> up taking up more vertical lines.  Which is also bad, because then we
>> can't see the full method, when it gets large.
>>
>>
>
> Yes, we should define a right margin size, and then migrate, steps by
> steps, all the code to this new coding style...
>
> And what to do with xml and java files ?

Actually, I was saying to not have an maximum enforced size.  If you
have problems with long lines, then configure your editor to deal with
the situation.

[Jacques Le Roux]

From: "Adam Heath" <[hidden email]>

> [hidden email] wrote:
>> Author: erwan
>> Date: Fri Mar 19 16:17:35 2010
>> New Revision: 925302
>>
>> URL: http://svn.apache.org/viewvc?rev=925302&view=rev
>> Log:
>> Correcting warning message on too long foreign keys
>>
>> Modified:
>>     ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java
>>
>> Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java?rev=925302&r1=925301&r2=925302&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java (original)
>> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java Fri Mar 19 16:17:35 2010
>> @@ -194,7 +194,8 @@ public class ModelEntityChecker {
>>
>>                          // make sure all FK names are <= 18 characters
>>                          if (relation.getFkName().length() > 18) {
>> -                            warningList.add("[RelFKNameGT18] The foregn key name (length:" + relation.getFkName().length()
>> +                            warningList.add("[RelFKNameGT18] The foreign key named " + relation.getFkName()
>> +                                            + " (length:" + relation.getFkName().length()
>>                                              + ") was greater than 18 characters in length for relation " + relation.getTitle() +
>> relation.getRelEntityName()
>>                                              + " of entity " + entity.getEntityName() + ".");
>>                          }
>
> Hmm.  What is the standard for overly long lines?  All on one, or
> wrapped?  The code base is not consistent in this manner.
>
> My opinion, is that when wrapping, it enforces a maximum size, that in
> all likely hood is less than what we developers actually have
> available.  This would then mean that we have tons of useless blank
> space on the right side of our monitors, and that the code would end
> up taking up more vertical lines.  Which is also bad, because then we
> can't see the full method, when it gets large.

On this aspect If we strictly followed the Java Conventions would be 80 chars max
http://java.sun.com/docs/codeconv/html/CodeConventions.doc3.html#382
I think it will be hard to get a consensus here. And even much, much harder to change existing code

Jacques

[Adam Heath-2]

Jacques Le Roux wrote:

> From: "Adam Heath" <[hidden email]>
>> [hidden email] wrote:
>>> Author: erwan
>>> Date: Fri Mar 19 16:17:35 2010
>>> New Revision: 925302
>>>
>>> URL: http://svn.apache.org/viewvc?rev=925302&view=rev
>>> Log:
>>> Correcting warning message on too long foreign keys
>>>
>>> Modified:
>>>    
>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java
>>>
>>>
>>> Modified:
>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java
>>>
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java?rev=925302&r1=925301&r2=925302&view=diff
>>>
>>> ==============================================================================
>>>
>>> ---
>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java
>>> (original)
>>> +++
>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java
>>> Fri Mar 19 16:17:35 2010
>>> @@ -194,7 +194,8 @@ public class ModelEntityChecker {
>>>
>>>                          // make sure all FK names are <= 18 characters
>>>                          if (relation.getFkName().length() > 18) {
>>> -                            warningList.add("[RelFKNameGT18] The
>>> foregn key name (length:" + relation.getFkName().length()
>>> +                            warningList.add("[RelFKNameGT18] The
>>> foreign key named " + relation.getFkName()
>>> +                                            + " (length:" +
>>> relation.getFkName().length()
>>>                                              + ") was greater than 18
>>> characters in length for relation " + relation.getTitle() +
>>> relation.getRelEntityName()
>>>                                              + " of entity " +
>>> entity.getEntityName() + ".");
>>>                          }
>>
>> Hmm.  What is the standard for overly long lines?  All on one, or
>> wrapped?  The code base is not consistent in this manner.
>>
>> My opinion, is that when wrapping, it enforces a maximum size, that in
>> all likely hood is less than what we developers actually have
>> available.  This would then mean that we have tons of useless blank
>> space on the right side of our monitors, and that the code would end
>> up taking up more vertical lines.  Which is also bad, because then we
>> can't see the full method, when it gets large.
>
> On this aspect If we strictly followed the Java Conventions would be 80
> chars max
> http://java.sun.com/docs/codeconv/html/CodeConventions.doc3.html#382
> I think it will be hard to get a consensus here. And even much, much
> harder to change existing code

The big problem with a max 80 width, is that when you have lines that
are indented quite a bit, you end up with the available content length
being 60, or 40,  or 30, or something.  And that makes the code that
much more harder to follow.

[Scott Gray-2]

On 19/03/2010, at 11:05 AM, Adam Heath wrote:

> Jacques Le Roux wrote:
>> From: "Adam Heath" <[hidden email]>
>>> [hidden email] wrote:
>>>> Author: erwan
>>>> Date: Fri Mar 19 16:17:35 2010
>>>> New Revision: 925302
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=925302&view=rev
>>>> Log:
>>>> Correcting warning message on too long foreign keys
>>>>
>>>> Modified:
>>>>
>>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java
>>>>
>>>>
>>>> Modified:
>>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java
>>>>
>>>> URL:
>>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java?rev=925302&r1=925301&r2=925302&view=diff
>>>>
>>>> ==============================================================================
>>>>
>>>> ---
>>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java
>>>> (original)
>>>> +++
>>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelEntityChecker.java
>>>> Fri Mar 19 16:17:35 2010
>>>> @@ -194,7 +194,8 @@ public class ModelEntityChecker {
>>>>
>>>>                         // make sure all FK names are <= 18 characters
>>>>                         if (relation.getFkName().length() > 18) {
>>>> -                            warningList.add("[RelFKNameGT18] The
>>>> foregn key name (length:" + relation.getFkName().length()
>>>> +                            warningList.add("[RelFKNameGT18] The
>>>> foreign key named " + relation.getFkName()
>>>> +                                            + " (length:" +
>>>> relation.getFkName().length()
>>>>                                             + ") was greater than 18
>>>> characters in length for relation " + relation.getTitle() +
>>>> relation.getRelEntityName()
>>>>                                             + " of entity " +
>>>> entity.getEntityName() + ".");
>>>>                         }
>>>
>>> Hmm.  What is the standard for overly long lines?  All on one, or
>>> wrapped?  The code base is not consistent in this manner.
>>>
>>> My opinion, is that when wrapping, it enforces a maximum size, that in
>>> all likely hood is less than what we developers actually have
>>> available.  This would then mean that we have tons of useless blank
>>> space on the right side of our monitors, and that the code would end
>>> up taking up more vertical lines.  Which is also bad, because then we
>>> can't see the full method, when it gets large.
>>
>> On this aspect If we strictly followed the Java Conventions would be 80
>> chars max
>> http://java.sun.com/docs/codeconv/html/CodeConventions.doc3.html#382
>> I think it will be hard to get a consensus here. And even much, much
>> harder to change existing code
>
> The big problem with a max 80 width, is that when you have lines that
> are indented quite a bit, you end up with the available content length
> being 60, or 40,  or 30, or something.  And that makes the code that
> much more harder to follow.

I hate trying to read wrapped lines, sometimes they improve readability like this:
UtilMisc.toMap(
    "item1", someLongMethodCall,
    "item2", someOtherLongMethodCall,
    "item3", yetAnotherLongMethodCall
);

but other than cases like that it usually makes things worse.

[Erwan de FERRIERES]

Le 19/03/2010 17:52, Jacques Le Roux a écrit :
> From: "Adam Heath" <[hidden email]>

> Jacques
>
>
>
i'm more for a 140 chars length size.

--
Erwan de FERRIERES
www.nereide.biz

[Adam Heath-2]

Erwan de FERRIERES wrote:
> Le 19/03/2010 17:52, Jacques Le Roux a écrit :
>> From: "Adam Heath" <[hidden email]>
>
>> Jacques
>>
>>
>>
> i'm more for a 140 chars length size.

This is you.  Someone else will will want 80, or 121, or, like me,
157.  There's no way you can pick some number, and make everyone
happy.  So, we shouldn't even bother, and not wrap long lines.

Each developer should then configure their editor to either fold the
lines for them, for display purposes only, or scroll as appropriate.