Topic on User talk:DKinzler (WMF)/Software Design Practices

Details I find confusing or disagree with

1
Thiemo Kreuz (WMDE) (talkcontribs)
"In the namespace hierarchy, it is discouraged for the code in a namespace to depend on code in any of its child (or more remote descendant) namespaces. This is primarily because it is expected for child namespaces to depend on code in their parent namespace, and having dependencies both ways would create a cycle."

Isn't this the wrong way around? Typically, I put the smaller classes that have less dependencies in the longer, deeper namespaces. These are then used by classes higher up in the namespace hierarchy, or even in adjacent branches.

"[…] is technical debt."

I wouldn't use the term "technical debt" here as if this would be the only possible definition of technical debt. I would argue that every code we write, no matter how well written it is, is technical debt. We introduced it, we need to maintain it, we need to constantly pay a little bit just to be able to have it around. The only code without any technical debt is code that doesn't exist.

"use of the global service locator should only be accepted as an intermediary step".

Thanks for this. The relevance of this can not be stressed enough. I see more and more code using service locators like nuts, but it creates almost all of the same problems as singletons a.k.a. global state.

"Generic configuration objects should never be injected into service objects, because doing so introduces a dependency on all settings."

That's a bit far stretched. When I have a class that accepts a Config object, and I clearly document the settings this Config object is expected to have, I can inject the global Config because it is compatible. This does not suddenly create a "dependency on all settings". I'm just passing useless settings the class doesn't use. As long as the class does not processes arbitrary settings, this is not a dependency.

"Subclassing should never be used just to share code."

Again, that's a bit heavy. There is not really a clear border between "this is clearly only done to share code" and when it is not. There is a gray area of personal style, opinions, and what a group of developers can agree on.

"Interfaces make bad extension points, because they cannot be changed without breaking implementations; use abstract base classes instead."

I find this somewhat weird to say. The only interface I know that is always acceptable does have a single, very trivial method. Using an abstract class instead of an interface does not make it easier to change the signature of this method. In case this is about the addition of more methods to the interface, I have to disagree. There should not be a need to "extend" an interface this way. It is a different interface then. Give it a new name, possibly make it extend the old one, or just copy the one trivial method header from the old one, and deprecate it.

Also, the basically only thing abstract classes do is enabling code sharing – which this very document forbids just a few lines above.

Reply to "Details I find confusing or disagree with"