[Proposal] Code standards change for more readability

[frankmayer]

Subject of proposal

I propose a change to code standards, concerning specific cases. This is intentionally posted here and not in the code standards repo, as I believe it will gain more attention and comments, here.

The changes affect:

• Multi-line implements in class declarations, see PSR-2 here: http://www.php-fig.org/psr/psr-2/#4-1-extends-and-implements
• Multi-line method arguments, see PSR-2 here: http://www.php-fig.org/psr/psr-2/#4-4-method-arguments

Why

It improves readability when either the implements or method arguments are many or the declaration in general is long.
Conforming to the current standard for method arguments, a method looks like this:

	public static function getContentLanguages($checkPublished = true, $checkInstalled = true, $pivot = 'lang_code', $orderField = null,
		$orderDirection = null)
	{

The same code is more readable like this:

	public static function getContentLanguages(
		$checkPublished = true,
		$checkInstalled = true,
		$pivot = 'lang_code',
		$orderField = null,
		$orderDirection = null
	){

Thank you for your comments and Happy New Year everybody! 🎆

[mbabker]

I'm all for it personally.

[jeckodevelopment]

in favour

[chrisdavenport]

If you have a method signature that requires splitting across multiple lines then I would consider it a code smell and you should fix the underlying problem rather than try to make it look pretty.

[frankmayer]

@PhilETaylor That's a valid argument, but what about people that don't have such IDE's. Or if on the road with a tablet or a phone and wanting to do some urgent necessary changes? ...just throwing some edge cases in here for the sake of argument.
@chrisdavenport Agreed, but let's make them pretty first, to be easier to read and then work on code smell. The work to reduce code smell is a much tougher job in most cases. Let's try to make things better in steps. Cannot magically clean up the code base in on go. ;)

[mbabker]

I hate it. This is why you invest in a decent IDE that makes it easier to view method signatures.

By that argument arbitrary line length rules are pointless too (like our 150 character max that has zero flexibility at all)

If you have a method signature that requires splitting across multiple lines then I would consider it a code smell and you should fix the underlying problem rather than try to make it look pretty.

Yes and no. You could probably refactor to use an array holding all of the arguments, but personally I consider THAT a code smell over lengthy method signatures (you no longer have a properly declared signature, it's for all intents and purposes a magical black box). Any way you spin it though, the intent here is for readable code and right now the way our multi-line method signatures split is not very readable. This is more of a convenience thing in the long run anyway (same as aligning => in multi-line arrays or the equals sign when you have variable assignments in a row).

As for the IDE argument, IMO it is unfair to state that all code changes must be made in a fully functional development environment which includes an IDE tuned to that environment. In Joomla there are a lot of contributors who by your claims aren't "professional developers" because they use the GitHub web interface to make their contributions, does that make what they offer less valid than a change proposed by you or me?

[Bakual]

It's still a good length because noody likes to read texts across the whole screen width :)

As for the multiline arguments, I don't care either way. However anything we do, should be supported by IDEs like PHPStorm so we don't have to manually adjust it each time.

[frankmayer]

The multi-line method arguments are supported by PHPStorm. The example above was formatted with PHPStorm 2016.3.2.

However PHPStorm currently has an issue with implements/extends, which is reported here: https://youtrack.jetbrains.com/issue/WI-24143 . So I guess, this might be a show-stopper for the multiline implements. Pls vote for it, if you're interested.

There is also an issue for PHPDoc formatting and the multiple spaces in between the tag parameters. If enough Joomla PHPStorm users vote for that, they might implement it faster... https://youtrack.jetbrains.com/issue/WI-24129

I don't know about the other editors. Anyone?

[photodude]

I believe this kind of multiline alignment can be "automatically fixed with PHPCS 2.x if we decide to make a change to the Joomla code standards. (Check out the current PHPCS 2/3 version of the code standards check with auto fixers https://github.com/joomla/coding-standards/)

[frankmayer]

@photodude That would be great! Lets see how it goes.

[tonypartridge]

I find this a bit of an awkward one. I agree with @PhilETaylor and personally use phpStorm it does a fantastic job of rendering the functions and making them readable. Also multiple lines means less code to fill the whole screen, so you are focusing on a tall but narrow screen with the above implementation, whereas most screens these days are widescreen which the exception of macs which tend to provide an exceptional amount of extra working space are the opposite way of this implementation. Therefore more code is in your view area which makes it easier to understand at a glance.

Even without an IDE you should be able to set your editor to highlight variables which make them stand out at a glance.

So I am therefore against this.

[mbabker]

If you focus solely on an IDE environment, then honestly a lot of the code style rules don't apply. Remember though, the code doesn't only exist in the IDE. It is browsed here on GitHub, edited here on GitHub and through other "non-professional" tools, and if I had configured the phpDocumentor job to do so the code could also be visible inline on the API documentation site.

I don't think there's anything wrong with disagreeing with the idea, but personally, I disagree with the notion that the code should only be formatted to be consumed in an IDE. It's about it being consistently reviewable regardless of the environment. I can participate in code reviews on a mobile device while traveling when looking at a well formatted code base, this particular change makes that experience a little easier (and ya that might be an edge case but it goes to demonstrate there is more to the code than whether or not you can drop it in PhpStorm, Eclipse, or Sublime and the tooling makes everything pretty and usable).

[brianteeman]

Closing this as all views have been expressed