PSR-2 whitespaces and parentheses

126 views
Skip to first unread message

Astinus Eberhard

unread,
Mar 31, 2015, 11:32:41 AM3/31/15
to php...@googlegroups.com
Hi everyone,

I was searching some other similar topic, but I couldn't find anything suitable, especially from recent threads.

When the surveys about PSR-2 begun, I was too young and incompetent to have a chance to take a part of it. Now I am using in my everyday work a whole bunch of tools, with CodeSniffer ahead, which have arisen on PSR-2.

My problem is, that (maybe through pedantic affection or exhausted eyesight) PSR-2 is so unreadable for me in some cases (see below).

I wrote my own patch to CodeSniffer (thanks Greg), which allows using whitespace(s) after opening and before closing parentheses in functions (methods) declarations and calls. Also I would like to allow whitespace(s) with unary operators, especially negation. This makes code much more readable (again - see below).

But now I am forced to use PSR-2, if I want to use my favorite (and honestly only reasonable) tools, like CS, PHP spec, etc.

My question is: why it is forbidden, why cannot be allowed (for one whitespace only at least), and (most bothering) why this very matter of PSR-2 standard did not find himself in the PSR surveys? (If I am wrong, sorry, please correct me, but I could't find it).

So please someone just slant over me, read my concerns and explain the reasons. Honestly, which of those logically-equivalent fragments is most readable? And what is so wrong with whitespace?

VERY BAD EXAMPLE, BUT CORRECT FOR PSR-2:
if (!$this->getSomething($that)->putSomewhere($arrPlaces[$this->getOrigin(Foo::TYPE_PHP)->getSymbol()])->isSuitable()) {
   
throw new RuntimeException(sprintf('%s[%d]: %s(): not fit', __FILE__, __LINE__, __METHOD__));
}

MUCH MODE READABLE, UNFORTUNATELY INCORRECT FOR PSR-2:
if ( ! $this->getSomething( $that )->putSomewhere( $arrPlaces[$this->getOrigin( Foo::TYPE_PHP )->getSymbol()] )->isSuitable() ) {
   
throw new RuntimeException( sprintf( '%s[%d]: %s(): not fit', __FILE__, __LINE__, __METHOD__ ) );
}

Even if I rewrote this, still PSR-2 is not as readable as could be (and what happened if I must use editor without code highlight?):

$strSymbol = $this->getOrigin(Foo::TYPE_PHP)->getSymbol();
$objNewPlace = $arrPlaces[$strSymbol];
$objSomething = $this->getSomething($that);

$objSomething->putSomewhere($objNewPlace);
if (!$objSomething->isSuitable()) {
    $strMsg = sprintf('%s[%d]: %s(): not fit', __FILE__, __LINE__, __METHOD__);

    throw new RuntimeException($strMsg);
}


But, hey, just few whitespace characters make a work:

$strSymbol = $this->getOrigin( Foo::TYPE_PHP )->getSymbol();
$objNewPlace = $arrPlaces[$strSymbol];
$objSomething = $this->getSomething( $that );

$objSomething->putSomewhere( $objNewPlace );
if ( ! $objSomething->isSuitable() ) {
    $strMsg = sprintf( '%s[%d]: %s(): not fit', __FILE__, __LINE__, __METHOD__ );

    throw new RuntimeException( $strMsg );
}

Matthew Weier O'Phinney

unread,
Mar 31, 2015, 12:56:00 PM3/31/15
to php...@googlegroups.com
On Tue, Mar 31, 2015 at 10:32 AM, Astinus Eberhard
<astinus....@gmail.com> wrote:
<snip>
> My question is: why it is forbidden, why cannot be allowed (for one
> whitespace only at least), and (most bothering) why this very matter of
> PSR-2 standard did not find himself in the PSR surveys? (If I am wrong,
> sorry, please correct me, but I could't find it).
>
> So please someone just slant over me, read my concerns and explain the
> reasons. Honestly, which of those logically-equivalent fragments is most
> readable? And what is so wrong with whitespace?
>
> VERY BAD EXAMPLE, BUT CORRECT FOR PSR-2:
> if
> (!$this->getSomething($that)->putSomewhere($arrPlaces[$this->getOrigin(Foo::TYPE_PHP)->getSymbol()])->isSuitable())
> {
> throw new RuntimeException(sprintf('%s[%d]: %s(): not fit', __FILE__,
> __LINE__, __METHOD__));
> }
>
> MUCH MODE READABLE, UNFORTUNATELY INCORRECT FOR PSR-2:
> if ( ! $this->getSomething( $that )->putSomewhere(
> $arrPlaces[$this->getOrigin( Foo::TYPE_PHP )->getSymbol()] )->isSuitable() )
> {
> throw new RuntimeException( sprintf( '%s[%d]: %s(): not fit', __FILE__,
> __LINE__, __METHOD__ ) );
> }

Honestly, this is a matter of personal preference; I find each equally readable.

--
Matthew Weier O'Phinney
mweiero...@gmail.com
https://mwop.net/

Astinus Eberhard

unread,
Mar 31, 2015, 5:06:44 PM3/31/15
to php...@googlegroups.com
Please people, think of it like it is very critical peace of code, and you have to fix the negation that cause bug on production. Maybe it's just a matter of personal preferences, but if it helps understanding logic and short-cuts work time for some number of people (if any), it might be considered as unnecessary limitation

Evert Pot

unread,
Mar 31, 2015, 5:17:48 PM3/31/15
to php...@googlegroups.com
On Tuesday, March 31, 2015 at 5:06:44 PM UTC-4, Astinus Eberhard wrote:
Please people, think of it like it is very critical peace of code, and you have to fix the negation that cause bug on production. Maybe it's just a matter of personal preferences, but if it helps understanding logic and short-cuts work time for some number of people (if any), it might be considered as unnecessary limitation

I think if you allow yourself to follow the rules of PSR-2, even in situations where it does not make sense, you are making a mistake.

PSR-2 is a reasonable set of defaults, but I think it's a mistake to take it as law. If you feel that you have a good reason to break the rules in certain situations where it greatly improves legibility, then the pragmatic decision would be to break the rule in that case.

Evert


Pádraic Brady

unread,
Mar 31, 2015, 6:52:58 PM3/31/15
to php...@googlegroups.com
Standards are there to be followed, however they are not necessarily a
snapshot of every single scenario. If you need to depart from them,
then do so. For the given example, if straying away from PSR-2 is
impossible due to automation, I'd tend to take the length as an issue
and start moving statements to indented newlines. Or perhaps extract
and isolate inner statements to preceding variable assignments that
make for a shorter, and sometimes more descriptive, if statement, at
the expense of there being an additional line or two of assignment
code.

Personally, I have no issues with some departures though I am quite
clearly biased. One of the consequences of CodeSniffer and PSR-2
coming together is that I've decided that PSR-2 is really a PITA for
actual work...when strictly enforced. As guidelines, they are
immensely useful. I use the subset that can be fixed via php-cs-fixer,
allow a bit of flexibility for the rest, and call it a day after that
;). I'm a programmer, and we're supposed to be lazy sods ;).

Paddy

--
Pádraic Brady

http://blog.astrumfutura.com
http://www.survivethedeepend.com

Mathew Whitney

unread,
Apr 1, 2015, 2:14:38 PM4/1/15
to php...@googlegroups.com
Since I started using CodeSniffer, I've been writing if statements with the Not operator like this:

if (! $objSomething->isSuitable()) {

    $strMsg = sprintf('%s[%d]: %s(): not fit', __FILE__, __LINE__, __METHOD__);

    throw new RuntimeException($strMsg);
}


Choice of font in the editor can make a significant difference in identifying certain symbols which seem to give some people a hard time (like the Not and Or operators), too.

The spaces after opening parens and before closing parens are something I only find helpful for readability if the parentheses contain multiple levels of parentheses (so most of your examples do not improve readability for me). I often find it more useful to do something else in these cases, or to rely on the editor's ability to match parentheses.

Over the years, though, I have found that whatever formatting I've been using recently is the most readable to me. Consistency within the project is far more important than any given standard. The benefits of the standard, in this case, are:
  • a number of tools already support it (e.g. CodeSniffer)
  • it is easier to read the code in other projects which use the same standard
  • it is easy to communicate the standards desired for the project
Reply all
Reply to author
Forward
0 new messages