RE: [phpGroupWare-developers] line lengths

[Dave Hall]

On Thu, 2008-05-01 at 21:03 +0200, Sigurd Nes wrote:
> 
> > From: Dave Hall address@hidden
> > Sent: 2008-05-01 18:40:58 CEST
> > To: address@hidden
> > Subject: Re: [phpGroupWare-developers] line lengths
> > 
> > On Wed, 2008-04-30 at 23:58 +0200, Sigurd Nes wrote:
> > > Chris Weiss wrote:
> > > > On Wed, Apr 30, 2008 at 3:28 PM, Dave Hall <address@hidden> wrote:
> > > >> On Wed, 2008-04-30 at 08:43 -0500, Chris Weiss wrote:
> > > >>  > On Wed, Apr 30, 2008 at 1:31 AM, Dave Hall <address@hidden> wrote:
> > > >>  > >  Yes, but also very long lines can indicate inelegant solutions.
> > > >>  >
> > > >>  > php is an inelegant solution.  more elegant than Basic though, where
> > > >>  > splitting up line is just plain tedious and limiting
> > > >>  >
> > > >>  > And just because it can be, doesn't mean it is, thus a target with 
> > > >> no
> > > >>  > hard limit.
> > > >>
> > > >>  Compromise is warn at 100 with no error?
> > > >>
> > > > yes, but i think Sigurd's point is that the number of warning and
> > > > errors caused by line length may be overwelming.  would be interesting
> > > > to see the difference
> > > > 
> > > 
> > > Recursive test for lengths on phpgwapi gives:
> > > A TOTAL OF 17248 ERROR(S) AND 7627 WARNING(S) WERE FOUND IN 477 FILE(S)
> > > 
> > > Errors being at length 100.
> > 
> > So what do you get using warnings at 100 and no error trigger which is
> > what was proposed?  Also are you sure you are only checking PHP files
> > which are ours, not 3 party libraries too? These should be excluded.
> > 
> 
> Seems to me that you are the only one that (really) want this - why
> not let it go?

I finally got around to testing this myself today.  As I suspected the
numbers were massively inflated by running the test over 3rd party code
too.

With warnings triggering at 120 characters, we get 1177 warnings, while
with them triggering at 100 characters we get 2573 warnings - Nothing
like the numbers above.

Now that we have accurate numbers in front of us, I think we can have a
proper discussion about the merits of the change.  I think for now we
should set the limit at 120, while encouraging people to keep it below
100.  The PHP_CodeSniffer test would still be set to warn at 120, with
no error trigger, for the time being.  We can review this later.

As a side note, running it over property turned up some interesting
results, such as a 240 liner from property/inc/class.uitts.inc.php:1488.
A lot of the time ternary operators ( test ? val_true : val_false ),
make the code harder to read, especially in the example above.  Useless
it is short (50 chars or so) and clear what it does, ternary operators
should be avoided.

Cheers

Dave