Topic on Talk:Best practices for extensions

Don't overuse private visibility in services

4
Summary by Legoktm

Removed the section.

Tgr (WMF) (talkcontribs)

I have difficulty interpreting this point. Classes should not be injected with many services? Services should not have many properties? Their visibility should be something other than private?

Thiemo Kreuz (WMDE) (talkcontribs)

The same idea is repeated further down in the document. I support the majority of this document, but this makes me sad. And I believe I can have an opinion on that because I still remember the time when I had the same idea. I was wrong: Services, classes, basically every code should only expose what it intentionally wants to expose. All kinds of interfaces (this includes class and function signatures) should be as narrow as possible, and only be opened up if an actual need to do so arises. Everything else should be private. Not protected, as protected is essentially "public to subclasses". Private by default.

Can we change the point to "Use private by default" or something like "Function, class and interface signatures should be as narrow as possible"? Or, if we can not agree on this, at least remove the point for now?

Legoktm (talkcontribs)

IIRC the idea was to make things protected rather than private so extensions have a chance to modify/extend things. I think it was a direct response to the "keep things as narrow as possible" concept because it makes external customization harder.

Thiemo Kreuz (WMDE) (talkcontribs)

To me, this sounds like a misconception. When I introduce private properties or methods while working on the implementation details of a class, I don't do this to somehow make it easier for future subclasses to "modify/extend things". What "things" would that be, anyway? I typically have no idea if a class will ever have subclasses, and what needs these will have. I design an interface for what I know needs to be shared with the world. But the implementation I write is just that, implementation. Making things "protected by default" is literally exposing implementation details.

Extension developers can easily submit a patch to make a method protected, if this is really what they need. Such a patch will typically (as I have seen it happen many times) start a brief discussion that will very quickly lead to a much better, much more sustainable architecture.