Skip to content

[11.x] more consistent and readable chaining#53748

Merged
taylorotwell merged 1 commit into
laravel:11.xfrom
browner12:AB-arrow-functions-and-multiline-chaining
Dec 6, 2024
Merged

[11.x] more consistent and readable chaining#53748
taylorotwell merged 1 commit into
laravel:11.xfrom
browner12:AB-arrow-functions-and-multiline-chaining

Conversation

@browner12

Copy link
Copy Markdown
Contributor

my goal with this commit is to make multiline chaining more consistent and readable. obviously there can be a subjective component to things like this, so I'll do my best to layout why I think these changes are objectively good.

I have limited this first proof-of-concept commit to chaining on new Collections, as they are a good simple example.

what did I change:

  • convert closures to arrow functions for very simple expressions
  • always begin the start of a chained method (->) on a new line
  • use a single indent from the first line on new lines

why I think this is better:

  • readability is better by not having multiple methods on a single line. no more hunting for the closing parentheses
  • each line serves a single grokable purpose. the first line instantiates the new Collection with some data. each following line runs an operation on that Collection
  • significantly improved GIT diffs. changes you see in the diff are more likely to be isolated to the purpose of the change
  • the single indent still gives us good alignment, without the pitfalls of trying to line it up with some arbitrary character or pattern in the starting line. it's very easy to apply consistently across many scenarios.

if this PR is accepted I will look into more places to apply this consistently, and into automation we can use for these rules as we move forward.

my goal with this commit is to make multiline chaining more consistent and readable. obviously there can be a subjective component to things like this, so I'll do my best to layout why I think these changes are objectively good.

I have limited this first proof-of-concept commit to chaining on new `Collection`s, as they are a good simple example.

what did I change:

- convert closures to arrow functions for very simple expressions
- always begin the start of a chained method (`->`) on a new line
- use a single indent from the first line on new lines

why I think this is better:

- readability is better by not having multiple methods on a single line. no more hunting for the closing parentheses
- each line serves a single grokable purpose. the first line instantiates the new `Collection` with some data. each following line runs an operation on that `Collection`
- significantly improved GIT diffs. changes you see in the diff are more likely to be isolated to the purpose of the change
- the single indent still gives us good alignment, without the pitfalls of trying to line it up with some arbitrary character or pattern in the starting line. it's very easy to apply consistently across many scenarios.

@shaedrich shaedrich left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some simplifications with first-class callable or traditional callable array syntax

Comment thread src/Illuminate/Database/Console/PruneCommand.php
Comment thread src/Illuminate/Database/Eloquent/Model.php
Comment thread src/Illuminate/Database/Query/Builder.php
Comment thread src/Illuminate/Database/Query/Grammars/PostgresGrammar.php
Comment thread src/Illuminate/Validation/Rules/File.php
@browner12

Copy link
Copy Markdown
Contributor Author

Some simplifications with first-class callable or traditional callable array syntax

you're welcome to try these changes in a separate PR, but I'm keeping things as simple as possible right now.

@shaedrich shaedrich left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could utilize higher-order messages

Comment thread src/Illuminate/View/Component.php
Comment thread src/Illuminate/View/Component.php
@taylorotwell taylorotwell merged commit e4a7357 into laravel:11.x Dec 6, 2024
@browner12 browner12 deleted the AB-arrow-functions-and-multiline-chaining branch December 6, 2024 20:34
browner12 added a commit to browner12/framework that referenced this pull request Dec 10, 2024
my goal with this commit is to make multiline chaining more consistent and readable. obviously there can be a subjective component to things like this, so I'll do my best to layout why I think these changes are objectively good.

I have limited this first proof-of-concept commit to chaining on new `Collection`s, as they are a good simple example.

what did I change:

- convert closures to arrow functions for very simple expressions
- always begin the start of a chained method (`->`) on a new line
- use a single indent from the first line on new lines

why I think this is better:

- readability is better by not having multiple methods on a single line. no more hunting for the closing parentheses
- each line serves a single grokable purpose. the first line instantiates the new `Collection` with some data. each following line runs an operation on that `Collection`
- significantly improved GIT diffs. changes you see in the diff are more likely to be isolated to the purpose of the change
- the single indent still gives us good alignment, without the pitfalls of trying to line it up with some arbitrary character or pattern in the starting line. it's very easy to apply consistently across many scenarios.
browner12 added a commit to browner12/docs that referenced this pull request Feb 14, 2025
use a single indent for chains on a new line, rather than trying to align it with some arbitrary pattern in the first line. this arbitrary alignment very easily gets out of sync as changes occur to the code, and often times the adjustments are not made to the chained alignment. the single indent alignment does not suffer this problem.

this is to bring us in line with laravel/framework#53835 and laravel/framework#53748

this commit only contains whitespace changes.

if accepted, I will make these adjustments to the remaining docs pages.
taylorotwell pushed a commit to laravel/docs that referenced this pull request Feb 14, 2025
use a single indent for chains on a new line, rather than trying to align it with some arbitrary pattern in the first line. this arbitrary alignment very easily gets out of sync as changes occur to the code, and often times the adjustments are not made to the chained alignment. the single indent alignment does not suffer this problem.

this is to bring us in line with laravel/framework#53835 and laravel/framework#53748

this commit only contains whitespace changes.

if accepted, I will make these adjustments to the remaining docs pages.
browner12 added a commit to browner12/framework that referenced this pull request Mar 11, 2025
this continues on some of my earlier formatting changes.

you can read laravel#53748 for my arguments why single indent is better.
taylorotwell pushed a commit that referenced this pull request Mar 12, 2025
this continues on some of my earlier formatting changes.

you can read #53748 for my arguments why single indent is better.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants