cancel
Showing results for 
Search instead for 
Did you mean: 

Change class members from 'private' to 'protected'

Change class members from 'private' to 'protected'

Feature request from hiephm, posted on GitHub Aug 19, 2016

@alankent Right now there are many classes that usually be inherited but all of their members are private (for example \Magento\Payment\Model\Method\Adapter). For child classes to use their parent members, we must re-declare constructor and all members again, which is not very convenience.

I know that OOP good practice is keeping member as much private as possible. But given the extensible nature of Magento, I think most of the private member can be safely changed to protected.

3 Comments
apiuser
New Member

Comment from maderlock, posted on GitHub Aug 19, 2016

I've come across this too, though more often it's just one or two things that are private making them hard to extend.

In addition, there are too many methods protected which means that plugins cannot be used with them, making the plugin mechanism rather less useful than maybe the creators expected.

likemusicdev
Senior Member

Related issue from Github: Use private instead of protected visibility

From  Technical guidelines : 

2.7. All non-public properties and methods SHOULD be private.

I still can't understand why not protected.

Sometime when developing extensions for magento I find issues in nested private method ($class->publicMathod()->privateMethod1()->privateMethod2()->buggyPrivateMethod3()). I'm talking about both php classes and js components. To fix nested buggy private method I should copy-paste many lines of code starting from public method to nested buggy. And after all new releases I should take care about copy-pasted methods if its not changed.

I think that make this nested private methods as protected would be less evil then thoughtless using private methods everywhere and cause tons of copy-pasted code from magento core files in 3-rd party extensions.

Am i wrong?

okopylova
Adobe Team

1. Protected methods allow/encourade inheritance, which we'd really like to avoid. Composition provides much more flexibility and so should be used instead.

2. Often "protected" is used just as a habit (going from Magento 1), and any "protected" member of a class becomes class's public API. Which means we can't change/remove it easily (first, we should deprecate it and only then, remove in one of the next minor/major releases). This makes life very hard. So, by default "private" should be used.

 

There are still places in Magento 2 that require inheritance, and in these cases we still have "protected", but this is something we refacror and want to eliminate in the future.

 

I hope it makes sense.