Problem/Motivation
There are cases where we use value objects, or where providing an interface for an otherwise one-off class will do more harm than good.
Benefits
If we adopted this change, the Drupal Project would benefit by ...
Three supporters required
- https://www.drupal.org/u/{userid} (yyyy-mm-dd they added support)
- https://www.drupal.org/u/{userid} (yyyy-mm-dd they added support)
- https://www.drupal.org/u/{userid} (yyyy-mm-dd they added support)
Proposed changes
Provide all proposed changes to the Drupal Coding standards. Give a link to each section that will be changed, and show the current text and proposed text as in the following layout:
1.https://www.drupal.org/docs/develop/standards/coding-standards#s-objects
Current text
Use an interface typehint for parameter and return types (not a class). Typehint the most specific interface that encompasses all possible parameter or return values. Do not use stdClass.
Proposed text
Use an interface typehint for parameter and return types where available. Typehint the most specific interface or class that encompasses all possible parameter or return values.
2. https://www.drupal.org/docs/develop/standards/php/object-oriented-code#i...
Current text
Use of interfaces
The use of a separate interface definition from an implementing class is strongly encouraged because it allows more flexibility in extending code later. A separate interface definition also neatly centralizes documentation making it easier to read. All interfaces should be fully documented according to established documentation standards.
If there is even a remote possibility of a class being swapped out for another implementation at some point in the future, split the method definitions off into a formal Interface. A class that is intended to be extended must always provide an Interface that other classes can implement rather than forcing them to extend the base class.
Proposed text
None. The text is to be deleted.
3. https://www.drupal.org/docs/develop/standards/php/object-oriented-code#h...
Current text
Type hinting
PHP supports optional type specification for function and method parameters for classes and arrays. Although called "type hinting" it does make a type required, as passing an object that does not conform to that type will result in a fatal error.
- DO specify a type when conformity to a specific interface is an assumption made by the function or method. Specifying the required interface makes debugging easier as passing in a bad value will give a more useful error message.
- DO NOT use a class as the type in type hinting. If specifying a type, always specify an Interface. That allows other developers to provide their own implementations if necessary without modifying existing code.
Example:
// Correct: function make_cat_speak(FelineInterface $cat) { print $cat->meow(); } // Wrong: function make_cat_speak(GarfieldTheCat $cat) { print $cat->meow(); }
Proposed text
None. The text is to be deleted.
Remaining tasks
\
Needs examples.
Create this issue in the Coding Standards queue, using the defined template- Add supporters
- Create a Change Record
- Review by the Coding Standards Committee
- Coding Standards Committee takes action as required
- Discussed by the Core Committer Committee, if it impacts Drupal Core
- Final review by Coding Standards Committee
- Documentation updates
- Edit all pages
- Publish change record
- Remove 'Needs documentation edits' tag
- If applicable, create follow-up issues for PHPCS rules/sniffs changes
For a full explanation of these steps see the Coding Standards project page
Possible follow up to remove to remove https://www.drupal.org/docs/develop/standards/object-oriented-code.
Comments
Comment #2
catchComment #3
xjmI think we should include of examples of a couple places an interface should be used and a couple where a class should be used.
Comment #4
alexpottCan we change
"Use an interface typehint for parameter and return types where available. Typehint the most specific interface that encompasses all possible parameter or return values."
to
"Use an interface typehint for parameter and return types where available. Typehint the most specific interface or class that encompasses all possible parameter or return values."
Comment #5
xjmAs part of this, we should also delete the redundant section at https://www.drupal.org/docs/develop/standards/object-oriented-code#hinting as it's the source of the "typehint interfaces only" coding standard, and write a CR when we adopt this I'd say.
Comment #6
catchComment #7
xjmCan we list out examples of when things should be hinted as classes or interfaces? And possibly reference a different section that documents some examples of when typehinting a class is actually preferable?
These are the other sections we need to change for parity:
and:
Obviously, both of these are kinda bad advice, since there are completely reasonable usecases to not have an interface. If we could instead write a short paragraph about when interfaces make sense and when they don't, we could get rid of the old docs.
(We also can get rid of the bits explaining what typehinting is; it's not some new weird concept and if someone doesn't know, there are better references for PHP basics.)
So let's rewrite those two sections as part of the proposed resolution.
Comment #8
catchI think we should remove https://www.drupal.org/docs/develop/standards/object-oriented-code entirely. The two sections in #3263602-7: Allow type hinting with classes where appropriate are either duplicating the point in the main document, or irrelevant.
Comment #9
quietone commentedConvert to new template
Comment #10
quietone commentedUpdated the issue summary with items in #7.
Comment #11
quietone commentedUpdate the issue summary #8.
That leaves adding examples as suggested in #3 and #7
Comment #12
kingdutchThe proposed improvement in #1158720: Improve text for parameter type hinting in function declaration shows that the type-hinting block is likely not only about interfaces (3 in this issue). As such I don't think it should be removed in its entirety, but we likely want to combine both changes. The wording proposed in that issue may already give more room for this issue since it qualifies interface usage with "if available", which I still think is what people should be doing.