[policy, no patch]: Atomic behavior objects

If a larger class gets split into smaller chunks that aren't coherent independently but have to be read together, then rather than four methods on the same class, you now have to deal with four methods on four different classes (and the additional three class names).

You are right, and it is an important point.
In my own projects I am following the approach proposed here, and I did sometimes run into cases like you describe them.

I would say there are different cases to distinguish:

1. The parts really belonged together, and we shouldn't have split them up.
   Maybe these would be "subatomic behaviors" then.
2. It was right to split things up, but not in the way it was done.
   Possible indicators:
   - Confusing method signatures.
   - Different classes whose (private) properties have copies of the same data.
3. We split things up correctly, but the parts are named and documented poorly.
   Perhaps PlaceholderStrategyInterface is such a case?
4. The reader is looking at it the wrong way.
   People need to learn to look at a small piece and accept it for what it is, before thinking of "the big picture".
   Likewise, one needs to be able to look at a higher-level class and accept smaller parts as "black boxes" if they were split out into dependencies.

My proposal is a change in philosophy, and the last point is essential for that.

I do not claim that everything is always possible to do in the way I propose here.
But it appears to me as if Drupal has entirely given up on even trying to make something like this happen.
For me this is a major turn-off for participating in any core development.

an abstract ideal of "one method, one class" for everything has been proposed before.

I am curious. Do you have a link?

If a larger class gets split into smaller chunks that aren't coherent independently but have to be read together, then rather than four methods on the same class, you now have to deal with four methods on four different classes (and the additional three class names).

Perhaps I should add:
Even now with "bigger" classes, if these are tightly coupled to other classes, you have the same problem, just on a bigger scale.
I often find it insufficient in D8 to look at just one class to understand what is going on.
And of course some big classes that have to be read together are worse than some small classes that have to be read together.

This does not invalidate your argument.
We still need to be careful how we split things up, and in some cases it might be better not to do it.

I wonder, is there a class in D8 core that is "big but beautiful", that you would proudly present at a PHP conference?
I cannot think of such a class myself, but I am asking this as an open-ended question, not a rhetorical one.

Adding to issue summary:

The starting point is always a single method. Additional methods should be the exception.
Additional methods should only be added, if they are relevant to every conceivable implementation of the main method.

@dawehner (#6): I will respond to this in more than one comment.

This particular usecase has driven #2810303: Reunite the router: One router to rule them all for example. The router system is something people don't extend, it just exists.

Thanks, it is always nice to look at examples!
I am new to this part, but here is what I see:

Old classes / services, now deprecated:

• route.matcher.final_matcher: Drupal\Core\Routing\UrlMatcher
  Class looks really small at first, but 2 base classes.
• router.matcher: Symfony\Cmf\Component\Routing\NestedMatcher\NestedMatcher
  One contract-public method, 4 additional public methods, no base class, 4 properties.
• router.dynamic: Symfony\Cmf\Component\Routing\DynamicRouter
  10 public methods, hierarchy of interfaces, 8 properties, no base class.
• router.no_access_checks: Symfony\Cmf\Component\Routing\ChainRouter
  (this service is not deprecated, only the class is replaced)
  10 public methods, hierarchy of interfaces, no base class.

It will take me some time to fully understand this.
But certainly these were not "atomic behavior objects".
Indeed I would have to look at multiple files before I get an idea what is going on.

More about this below.

New class / service:

• router.no_access_checks: \Drupal\Core\Routing\Router
  10 public methods, has UrlMatcher from above as a base class.

Maybe this change gets rid of some overly generic and flexible code, and instead hard-codes some of those things.

But I still need to look at multiple files to understand what's going on.

More observations

Symfony\Cmf\Component\Routing\NestedMatcher implements both FinalMatcherInterface and UrlMatcherInterface. So which of those is what it wants to expose to the public?
(this applies both before and after the commit)

Some properties of UrlMatcher are updated for every request or route collection that is being matched.
$this->routes, $this->context, $this->allow, $this->request, perhaps others?
This makes the class mutable / stateful, and thus makes it harder to see what's going on.

With immutable classes, all this information would have to be passed around as parameters, which would make it more transparent.
Or alternatively, one would create new short-lived instances with this extra data.

UrlMatcher also has a $this->currentPath property, which is set in the constructor. I wonder why? Shouldn't this depend on the request, which is only known later?

Conclusion

Using one hard-coded custom implementation instead of composing from several generic and overly dynamic implementations can sometimes reduce complexity.
Perhaps this is what happened here, but it would take me more time to determine that.

But certainly this system is and was a very long shot from qualifying as "atomic behavior objects", be it before or after the change.
To get there, you would have to:

• Avoid parent classes. (*)
• Properties should be private. Those base classes that remain should be abstract, and have a clearly defined API for extension, not arbitrarily expose everything.
• Ideally, only one interface per class. (*)
• Ideally, an interface should not have a parent interface. (*)
• Ideally, one method per interface. (*)
• No mutable properties. Everything that is different per request should be parameters, or live in temporary objects.
• No setters. Mostly constructor injection, perhaps some withers.
• A clear naming scheme, that tells about "what" (the interface name) and "how" (how it is different from other implementations). Properties for injected behaviors should also, in the general case, repeat the interface name.

Basically all the stuff I wrote in the issue summary.

(*) These "requirements" are a starting point. If we run into a brick wall with one of those, we can take some liberties. But don't give up too early.

Would it be possible to implement the routing system like this? Would it be desirable?
I bet it would be possible.
But a lot of functionality in Symfony is already not built in the way I describe, so there are limitations.
And of course there would be understandable hesitation to touch a system that already works as it is.

I am not sufficiently familiar with the routing system, so for now I cannot present a clear vision how this would look like.
The starting point would be identify the main behavior(s) that the rest of Drupal expects from the routing system, and then work from there.

A while ago I saw a talk at a functional programming conference: https://www.youtube.com/watch?v=XpDsk374LDE and it is interesting how they argue for not splitting up things into million of files. In JS this is quite common as individual files can intersect with others. At least that isn't a problem in PHP.

The guy says it is not necessary to split stuff into files, because ELM somehow prevents variables from "leaking" between functions (or classes? How does ELM work?).
So, the equivalent in PHP would be to put multiple small classes with private properties into the same file.
The equivalent would NOT be to have one big class with protected properties, which are visible throughout the class and its entire hierarchy.

For us, there are other reasons to put classes in separate files:
- autoloading.
- matching of class name to file name to find things more easily.
- version control: having stuff in separate files reduces conflicts.

@dawehner (#6). Now to your main argument.

If you have a piece of software and you want to figure out what is going wrong, having lass less fewer classes helps, as you need to have less of the overall picture in your head at the same time.

In a well-designed architecture, you can look at one small or "small enough" artifact (class, method), without worrying about the rest.
Everything outside should be "someone else's problem".

To make this possible:

Implementation of dependencies: Not my problem!

Interfaces for injected dependencies, and the respective properties, should have really clear names, and the exposed signatures have to be obvious and well-documented enough that you don't have to go looking at implementations.

Example (a negative one): In Symfony\Component\Routing\Matcher\UrlMatcherInterface it says "@return array An array of parameters", which makes me wonder "What is a parameter? Let's look at an implementation and find out." So, a poor doc description can already make this fail.

Example (a positive one): Drupal\Core\Extension\InfoParserInterface takes a file name to a yml file, and returns the parsed data as array, according to yml spec and to Drupal's info file schema. How it does that, I don't care.

We could argue whether it would be better to return a value object instead of an array, but imo the array is good enough for this component, perhaps even better than a value object.

In "SOLID" terminology this is the "Depend upon abstractions" part.

Calling code: Not my problem!

Likewise, a nicely-decoupled behavior object should not have to care how it is called.
It does its job according to contract, and goes home.

Again the InfoParserInterface is a nice example. It does not care where the file name comes from. It does not care where the yml file contents are used.

Now we could argue that the info parser and calling code are somehow coupled via the info schema.
The docs on the info parser are quite verbose with the schema details, and all of this is part of the contract.
Or is it really? The info parser just checks some required keys and replaces VERSION with the actual version of core, but does not enforce anything else of the schema. This would be the calling code's responsibility.

We could also argue whether the static cache and the inheritance in InfoParser are a good idea, but again this is out of scope here.

About "state"

For example if you keep state around, having less places to look out for it really helps.

Well, if you have immutable objects, you don't have state.
If you still need to manage state somewhere, you put it in a distinct layer, which only manages the state and does nothing else.

If the state changes, then some immutable objects might become outdated, and need to be thrown away and replaced (or kept somewhere until the state changes back). This is actually a nice way of managing such changes, because it keeps the statefulness out of the class that does the main work.

E.g. code I recently looked at was theme registry (Drupal\Core\Theme\Registry and Drupal\Core\Utility\ThemeRegistry).
See #2957451-11: Some parts of Theme\Registry are written as if they support multiple themes (initial premise in the issue title might be wrong, I think)
The Theme\Registry, each time something is rendered, checks $this->themeManager->getActiveTheme(), which can sometimes change mid-request. This caused me headache in some other issues.

By splitting the state into a distinct layer, we would have one immutable (except cache) Theme\Registry object per theme, where all the logic would happen, and then one toplevel object which would always pick the correct Theme\Registry object for the current theme.

More on this.

If you have a piece of software and you want to figure out what is going wrong, having lass less fewer classes helps, as you need to have less of the overall picture in your head at the same time.

I should add, as I did in #4:
For most of the classes in the current architecture, even if they are big, and naturally fewer than if we would split them up, it is not enough to look at just this class.
Usually,
- the class you look at has some part of the stuff you care about.
- it also has a lot of stuff that you do not care about at this time.
- some other part of what you care about is elsewhere. Perhaps in the base class, or somewhere completely separate.
- the class is coupled to other classes in various way, which might not have what you care about, but you still need to look at, to see what is going on.

So, imo, the current architecture does not at all solve the problem you describe.

----

This said, I agree that what you describe can occur with small fragmented classes.
I could point at examples in my own modules where I have over-fragmentation, and the distinct parts are not really independent, even though they have formally clear interfaces. E.g. in xautoload, I try this to manage different conditional initialization phases at bootstrap and on first cache miss. It is weird and, you could say, over-engineered. But luckily it works.

But usually the answer was not "bigger classes". Often the answer is that I did a poor job identifying what are "atomic" behaviors - see #3.

In some cases, if an interface is too weird, e.g. if its parameters and return value only make sense in a specific context, one can use a base class with abstract protected method instead of an interface. Here we can be sure that the method will only be called in the context of the base class.

I found some interesting articles related to this.
Not directly about "atomic behavior objects", but about "Rich Domain Model" (RDM) vs "Anaemic Domain Model" (ADM).
I kind of heard these terms before but only now have I done my homework and actually read about it.

The main question there is: Where should (domain) logic and behavior live?
A) In the domain objects / entities. (RDM)
B) In some "bulky" services. (ADM)
C) In many tiny services. (ADM)

I would say the "atomic behavior objects" fit more easily into an ADM than into an RDM.

In the RDM, domain objects (entities) contain data and logic.

With an ADM, domain objects (entities) are light-weight, have no or very few dependencies, and mostly just hold their own data.
Most of the logic, and also the loading and saving of entities, happens in dedicated services.

People who don't like the ADM claim that you get ugly, bulky services.
I don't see why this needs to be the case, and indeed "S" on "SAPM" (see below, who is this?) argues that you can and should split the services into small ("atomic", I say) components.

One article also claims that in the RDM you use value objects as building blocks for the entities, whereas in the ADM you would put all the data in the entity. Again I don't see why this needs to be the case, nothing stops you from using value objects in an ADM.

Martin Fowler (in 2003) argues that the ADM is not really object-oriented.
Personally, I think we need to let go of the idea that objects have to map 1:1 to conceptual entities and their behaviors. I think it's a trap to think this.

Friends of the ADM (e.g. "S" on "SAPM") claim that with an RDM you get really nasty big entities / domain objects.
Maybe they exaggerate a bit, and fail to mention that you can split domain objects to some degree.
E.g. one bad example for an RDM makes the customer entity responsible for various tasks, which could live in a shopping cart entity, or a checkout object, which might still be considered domain objects.

But I would argue that there are natural limits to how far you can split up domain objects, whereas behavior objects or services can be split up down to the atomic level if we want to.

Articles about Anaemic Domain Model

2014, "S" on "SAPM" (does this person have a name??)
The Anaemic Domain Model is no Anti-Pattern, it’s a SOLID design

The Anaemic Domain Model better supports Automated Testing

Refactoring the RDM to apply SOLID tends to result in an ADM

By exploring the implementation of a straightforward example, we have observed that an Anaemic Domain Model better adheres to the SOLID principles than a Rich Domain Model.

2017, Kamil Berdychowski
Domain-Driven Design vs. anemic model. How do they differ?

2017, Sebastian Gebski on no-kill-switch
Once an anti-pattern: Anemic Domain Model

2010, Bozho's Tech Blog
ON DOMAIN-DRIVEN DESIGN, ANEMIC DOMAIN MODEL, CODE GENERATION, DEPENDENCY INJECTION AND MORE…

2003, Martin Fowler
AnemicDomainModel
He does not like it..

The fundamental horror of this anti-pattern is that it's so contrary to the basic idea of object-oriented design; which is to combine data and process together.

How is this relevant?

As said above, the "atomic behavior objects" I propose here fit more nicely into an "Anaemic Domain Model" than into a "Rich domain model".

Currently I would say we are somewhere in between, or have the worst of both worlds.
We have bulky domain objects (entities), and bulky services.
(We also have some ok-sized objects, but not as many as I'd like)

The goal of this issue is to gradually and carefully move logic and behavior out of either of those, and into atomic behavior objects.
Or at least, decide whether this is a desirable goal.

Are "atomic behavior objects" services?

They can be.
Sometimes we have "local" behavior objects, which are not meant to be services.

Procedural? Functional?

(see also "Callables/functional?" in the issue summary)

Some people argue that the ADM takes us back to procedural times.
The same could be argued about the "atomic behavior objects" that I propose in this issue.

In a way this is true:
When I look at D7 code, and think about how to turn this into classes and interfaces, I tend to think of one class per function, rather than putting multiple things together.

I think there is a misconception about classes as "a container for more than one function", which leads to the idea that we have to put more stuff into it to justify it being a class.
The real benefit of a class instance is that it can contain data and dependencies on a formalized way, and that it can be replaced by something else with the same interface. This benefit applies even if it has just one method.

We get some of these benefits with closures, but in PHP there are no enforced contracts for closures, so this is a dead end imo.

Are "atomic behavior objects" services?
They can be.
Sometimes we have "local" behavior objects, which are not meant to be services.

I should add: Some of our services need to handle state changes.
E.g. the theme registry and other theme-related components need to be able to switch the active theme.
Such services would not be considered clean behavior objects.
But a lot of the logic, e.g. theme registry discovery, can be moved into dedicated objects, leaving a top level which deals with state changes.

I searched for "aenemic" in the issue, queue, and found only one result!
What does this tell us about our community?

@GroovyCarrot in #2009958-14: Introduce EntityStatusInterface to standardize how entities can be disabled

Many interfaces that just allow you to pass in arbitrary values to set on the domain, isn't really an abstraction of anything, and just results in objects that are bags of setters and getters; this leads to anaemic domain design, which just leaves massive scope for error.

I quote this because it is a recurring argument against the ADM.
The situation can happen if we would move all logic out of a domain object / entity. So, it is very relevant to this issue.

I think the answer is: Even if we move a lot of behavioral code out of domain objects, they still need to be responsible for their own integrity as an object. Any public mutation method on an object needs to transform it from one valid state into another.

I searched for "aenemic" in the issue, queue, and found only one result!

More results for "anemic".
https://www.drupal.org/project/issues/drupal?text=anemic&status=All&prio...

I somehow doubt that this is going anywhere as a general policy.
Lets see if there are any responses from others, otherwise ok to see it closed after 3 months.