Jump to content

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

Smalyshev (WMF) (talkcontribs)

There shall be no cyclic dependencies between modules, public or private. That is, for code to be split into a separate module, all of the dependencies it has on the code that remains in the old module needs to be removed.

Not sure I understand this. If I split code from module A to module B, why B can't depend on A? Surely I don't want two-sided A<->B, but I don't see why one-sided dependency on old module is bad? This obviously makes no sense if the old module was core, since every extension would depend on core. But even between two extensions - if I had search functionality in Wikibase and I took it out to separate module, that module would still depend on Wikibase.

Maybe it was meaning the reverse - that the code that remains in old module can not depend on the code being moved out? That would make sense, if we don't consider hooks and other API overrides (such as configurable factories, etc.) a dependency.

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 looks a bit too limiting. E.g. if I have Search class that has fields and queries, I would have Search\Field and Search\Query namespaces where field and query related interfaces would live, and then the class that implements the main engine may live in main Search namespace, but still use interfaces and possibly implemented code from Search\Field and Search\Query. Not being able to do this requires hierarchies to be completely flat.

Most objects should be either immutable values or "stateless" services

Not sure what is meant by "most". Given that PHP has no concurrency, I don't see why we should emphasize immutability that much.

Service objects do not have to be truly stateless, they may maintain "hidden" state, that is, use things like caching or lazy initialization.

I think here it should be included that if the state is hidden, there should be a way to either clear it or at least reset it to a known state, otherwise such class is untestable.

Reply to "More notes"