[New Rule] Method chaining in class design MUST be avoided

[lenaorobei]

Rule

Method chaining in class design MUST be avoided.

Reason

Source: Magento Technical Guidelines.

According to technical guidelines:

Application SHOULD be structured in compliance with the CQRS principle.

Good explanation of why method chaining contradicts this principal was described by Martin Fowler in the "Domain-Specific-Languages" book:

Command-query separation is an extremely valuable principle in programming, and I strongly encourage teams to use it. One of the consequences of using Method Chaining in internal DSLs is that it usually breaks this principle - each method alters state but returns an object to continue the chain. I have used many decibels disparaging people who don't follow command-query separation, and will do so again.

Suggested Implementation Direction

Detect return $this; in methods.

[orlangur]

@lenaorobei I do understand that such "useful" sniff is quite easy to implement but it is firstly not applicable to Magento codebase and secondly is wrong in general. For example, returning $this is perfectly valid in builders.

Could you please point out where its approval comes from? Like architectural meeting notes or something.

[lenaorobei]

@orlangur the fact that the rule is accepted doesn't mean that implementation is accepted. The example I provided is just the suggested direction where to start the implementation. I'll fix the issue content.

Could you please point out where its approval comes from?

We have an agreement with core team that every rule from Magento Technical Guidelines is automatically accepted.

[chaikovskyia]

@lenaorobei , why the rule is marked as MUST while applying CQRS principles is recommended (SHOULD)? Also does that mean that data storage classes and interfaces will not have setter methods anymore as currently there too many methods that are modifying state of an object?

[lenaorobei]

Hello @chaikovskyia. This rule is the paragraph 2.15 of the Technical Guidelines.

In case of any concerns regarding this rule, please crate an issue in the magento/architecture repository. Looks like this rule requires additional explanation and some exceptions.

[lenaorobei]

Cannot be implemented with PHP CodeSniffer.

[orlangur]

@lenaorobei why we cannot have it with exceptions for particular class types (extending DataObject, for instance)?

[lenaorobei]

@orlangur we need to enforce this rule and can discuss use cases and appropriate tool.
We cannot use PHP CodeSniffer for that for sure, because this tool manipulates with file tokens only and does not know about parent classes, etc.