The Doctrine Core Design: A Discussion on Private Methods, Properties, and Final Classes

[mgamadeus]

Hello everyone,

I've been working extensively with Doctrine as part of a Symfony project, and I've tailored many components to fit my specific needs. My project boasts a custom-built Entity/EntityGraph system, with repositories that fetch data from both databases and external APIs. This data is then harmonized at a higher level of abstraction, the Entity level. While I utilize Doctrine for the database layer, I often find myself not using it in the way it was originally intended.

One of the recurring challenges I've faced is the design pattern adopted by many core components of Doctrine. Components like the QueryBuilder are rife with private methods and properties. Additionally, classes such as Query are marked as final, which significantly inhibits extensibility.

Here are some of the challenges and arguments against this practice, especially from the perspective of my Symfony project:

Cascade of Code Duplication: Due to the restrictive nature of final classes and private members, I've often found myself copying core classes to adapt their behavior to my needs. For instance, since I cannot extend the final Query class, I had to duplicate it. This led to a cascade effect where I also had to copy the Parser class, given that its constructor requires a Doctrine\ORM\Query parameter.

Inhibited Extensibility & Adaptation: Doctrine's design sometimes feels like it's working against the grain of my project's requirements. The inability to extend core components has often limited innovative solutions, especially when integrating with my custom Entity system and external APIs.

Testing & Maintenance Overhead: With the need to duplicate core classes, testing becomes more complex. Maintaining these duplicated classes also introduces potential inconsistencies and increases the technical debt of the project.

Integration with Symfony: Symfony projects often require a certain level of flexibility and adaptability, especially when integrating with other systems or custom-built components. The restrictive nature of some Doctrine components can sometimes clash with the modular and extensible ethos of Symfony.

Over-Encapsulation & Refactoring Challenges: While encapsulation is crucial, the overuse of private properties and final classes in Doctrine can make the codebase rigid. As the software ecosystem evolves, adapting and refactoring parts of the Doctrine integration in my Symfony project becomes more challenging.

I understand the reasons behind using final classes and private members in Doctrine, such as ensuring stability and preventing unintended side-effects. However, from the perspective of a Symfony developer trying to integrate Doctrine in non-traditional ways, these design choices can sometimes feel limiting.

I'm eager to hear thoughts from the community. Has anyone else faced similar challenges when integrating Doctrine with Symfony or other frameworks? Are there workarounds or best practices that you've found helpful?

Looking forward to a constructive discussion!

[adriankott]

When integrating Doctrine in Symfony, I've encountered challenges similar to yours. These limitations can be quite restricting, especially when attempting to integrate it unconventionally. I completely understand your concerns and believe they are an important issue that requires attention. Hopefully, the community will collaborate to develop a stable yet more flexible solution.

[greg0ire]

How did you hear about this discussion?

[Schnorts]

Hello everyone,

I wholeheartedly agree with the points raised by the original poster, especially when it comes to the challenges of integrating Doctrine with Symfony projects. The use of final classes and private methods/properties does indeed limit the extensibility and adaptability of the framework, making it difficult to tailor it to specific project needs.

I've also faced similar issues in my projects, where the restrictive design patterns have led to unnecessary code duplication and increased maintenance overhead. This is particularly problematic when trying to innovate or integrate with other systems.

I understand the rationale behind using final classes and private members for stability and to prevent unintended side-effects. However, I believe there could be a middle ground that allows for both stability and flexibility. Perhaps introducing more protected methods or making some classes non-final could be a step in the right direction.

This seems like a relatively straightforward change that could significantly improve the developer experience without compromising the integrity of the Doctrine framework.

[greg0ire]

How did you hear about this discussion?

[greg0ire]

Perhaps introducing more protected methods or making some classes non-final could be a step in the right direction.

Yes, maybe, but by default, everything should be as closed as possible IMO. Once something is not final, you cannot make it final anymore, it's a breaking change. Making a class not final or a property protected should be justified with a very good reason, and might not be the best solution everytime, so let's not do it blindly.

[rankingcoach]

I wholeheartedly agree with the points raised here. As a developer who has also worked with Doctrine in Symfony projects, the restrictive nature of final classes and private members has often posed challenges for me. It's essential to strike a balance between encapsulation for stability and flexibility for innovation. Given that many of these concerns can be addressed with relatively simple changes, such as making some classes non-final or changing private members to protected, I believe it's a reasonable request. This would greatly enhance the adaptability of Doctrine without compromising its core principles. Let's prioritize developer experience and flexibility while maintaining the integrity of the framework.

[greg0ire]

How did you hear about this discussion?

[rb-key-accounts]

it was posted in a private discord channel. Marius was working at our company built a commercial entity framework which we now use internally and asked if we could upvote since extensions of the Doctrine core classes are almost impossible. honestly i can understand him, as i saw this issue also in other projects when extensions of external components were necessary. Having all of them spicked with private properties and final classes is a bit of overprotection.

[greg0ire]

Having all of them spicked with private properties and final classes is a bit of overprotection.

Making a class not final or a private property private is not a breaking change. The opposite is not true. The right way to do this is to complain in an issue, with a well-explained problem, so that we can consider opening classes, adding interface, making properties protected or adding extension points on a case by case basis.

[romeotamas]

I totally agree. Making Doctrine more flexible, especially for Symfony projects, sounds like a good move. Some simple changes could make a big difference. Hope the Doctrine team can consider this!

[greg0ire]

How did you hear about this discussion?

[romeotamas]

I just found and read the article on the https://github.com/doctrine/orm/discussions board. It is straightforward.

[greg0ire]

Had no idea the community was so active. Did you get a notification maybe?

[romeotamas]

Yes, we got a message in our internal channel about Marius' discussion. He is working very hard on our company built solution using Symfony. Did we do something wrong? Why are you asking?

[greg0ire]

Because it looks super suspicious, but you did nothing wrong, don't worry :)

[rb-key-accounts]

I totally get what you saying. Me too have work with Doctrine many times and face same problems. Final classes and private things make hard to change and adapt. If Doctrine more flexible, it be easier for all of us. They should really think about this. It not hard to make this change. I support you!

[greg0ire]

How did you hear about this discussion?

[greg0ire]

ensuring stability and preventing unintended side-effects

Everytime somebody rants against final and private, they fail to mention the actual reasons we do this. These are not the main reasons, or at least, it's not described clearly enough. You can find more interesting reasons here: http://fabien.potencier.org/pragmatism-over-theory-protected-vs-private.html

TL;DR: if something is badly designed, the solution shouldn't be to extend and override, but to report the issue and get the design changed. final and private encourage that, because they make it painful to fix stuff in your app, just like you described.

Also, it allows us to do refactoring without breaking changes. When a class is extensible, many more changes become breaking changes.

As I'm writing this, there are upvotes and agreeing answers raining with suspiciously frequent timing… Do you guys know each other?

[greg0ire]

@mgamadeus did you create fake accounts to upvote yourself? They have little to no contribution activity, and are not responding.

[mgamadeus]

:D no!
I pasted it to a private discord group and asked people if they can upvote to raise the visibility of the topic.

[Hanmac]

but to report the issue and get the design changed.

says the Repo with over 1000 open Issues and 250 unmerged/unclosed pull requests

it seems your suggestion with just report and get it changed doesn't seem to work as you think?

[stof]

Well, "get it changed" also involves being careful about how this gets changed regarding the BC impact.

But also look at all the merged PRs. With everything being protected everwhere, most of those would not be possible (or each release would be a major one)

[greg0ire]

Well it's not very effective since there is only one notification in the inbox of maintainers that haven't checked this out yet for the whole discussion, regardless of the number of upvotes. Please don't do that next time, because now the discussion is biased, since people commenting here are not random, but people that you knew experienced the same pain as you did. Feel free to open separate discussions for every issue where you were first to copy/paste code. Maybe we can improve Doctrine's design to make it easier to solve them.

[dkarlovi]

I'm using Doctrine in a way it was not intended, including but limited to using its features to talk to external APIs and turn on/off lights in my garage.
To my surprise, the design of the code is such almost as if it's actively trying to prevent me from doing that, which feels disrespectful toward my project about, again, turning my garage lights on/off using Doctrine ORM.
Since I have some guests coming in tomorrow, I hope we can get this fixed by this evening since I'm not walking there and turning it on manually. Let's get it changed.

You're using Doctrine ORM as a generic framework, it's not meant to have

repositories that fetch data from both databases and external APIs

Doctrine ORM is about database storage and mapping data to/from PHP objects (Entities) into database rows, that's basically it. It's best practice to not even use those entities in the rest of your app, you convert your own DTOs into entities and entities into DTOs, you've done the exact opposite and made everything and entity, even stuff which was never ever meant to be one.

Are there workarounds or best practices that you've found helpful?

Yes, not doing that will help immensely so I'd start slowly not doing it and then speed up the process by 70%.

[greg0ire]

if i extend classes from an external lib, i am well aware, that i have to keep things up to date

Maybe that's just you. Some other people will be extremely pissy when reporting such breaking changes in a public API, and sometimes rightfully so.

[dkarlovi]

for example if i want to extend the EntityManager, QueryBuilder and Query class to add some additional functions, what is so bad about this?

You're not meant to extend them, you're meant to decorate them. Adding features by "extending classes and adding some additional functions" is not how object oriented programming is meant to work.

[rb-key-accounts]

as an example, Eloquent has not a single class marked as final
https://github.com/search?q=repo%3Ailluminate%2Fdatabase%20final%20class&type=code
and in the whole codebase only 3 methods marked as private
https://github.com/search?q=repo%3Ailluminate%2Fdatabase+private&type=code
not one single private property

[greg0ire]

Well then use Eloquent? Problem solved!

[dkarlovi]

Eloquent has not a single class marked as final

So?

[derrabus]

Thank you for opening this discussion.

Since we're all exchanging our own experiences here, let me share some of mine. I have done quite a lot of projects in the past that used the Doctrine ORM. Apart from classes that are explicitly meant to be extended (like repositories for instance), I haven't felt the need to extend any classes that the ORM ships. None. We're discussing a problem here, that does not exist in my world.

Why's that? Have the projects I have worked on been too simple? I doubt that. Am I falling back to plain SQL too soon if the ORM feels like a bad fit for a problem at hand? Maybe. But I think, the most important part is this one:

While I utilize Doctrine for the database layer, I often find myself not using it in the way it was originally intended.

Why.

Doctrine's design sometimes feels like it's working against the grain of my project's requirements.

Are you sure, it's Doctrine's design working against your requirements or is it you deliberately working against Doctrine's design?

I mean, I use a specific library because it fits my requirements. If it doesn't, my options are…

• look for a different library
• try to adjust my requirements
• write my own library

In my projects, I use a lot of libraries that I would've designed differently. But if they work for me, I accept the fact that other developers chose a different design approach and move on.

And if they wouldn't, I wouldn't use them. My rule of thumb here is: if the code that I need to write and maintain in order to get a third-party library to work is at least as complex as my own replacement for that library, I don't use that library.

A well designed library defines documented and well tested extension points. I you stick to them, you can safely upgrade the library and if something goes wrong, maintainers or other users of that library can provide assistance because they can reproduce your problem easily.

Now, if a library just declares all classes as extensible and all properties and methods protected, what happens is: Everything becomes an extension point by accident. Sounds great?

Testing & Maintenance Overhead: With the need to duplicate core classes, testing becomes more complex. Maintaining these duplicated classes also introduces potential inconsistencies and increases the technical debt of the project.

Yes, let's talk about testing and maintenance. When maintaining a library, one of my goals is to remain compatible with code that has already been written against my library. This is why I'm testing all extension points and why I'm extra careful when changing APIs that could've been consumed or extended downstream.

Now, if everything is an extension point, I have several problems:

• The amount of tests that I need to cover my extension points would be insanely high.
• Every new feature contribution is a potential minefield because it might reorganize internal methods that might've been overridden downstream.
• Potential contributors are scared away by the sheer complexity of the BC layer that we have to maintain.

So, what I could do about this is I could flag all classes as soft-final (/** @final */) and all protected methods and properties as /** @internal */. This would tell everyone that I didn't intend these artifacts to be extension points and allow me to change them whenever I want because everyone who extends or uses those artifacts would know the risks, right? Problem solved? No, because at the same time you complain that…

As the software ecosystem evolves, adapting and refactoring parts of the Doctrine integration in my Symfony project becomes more challenging.

And I guarantee you: If we open up everything for extension, this exact problem will bite you even more. Because you are 100% going to extend stuff that we've never anticipated.

The result of that is that many projects will face major compatibility issues whenever the ORM issues a new release and as a consequence postpone upgrading the ORM. And if those projects need a bugfix, e.g. for compatibility with newer PHP or Symfony releases, they'll open issues asking us to backport fixes to some ancient branch because they haven't upgraded yet. Fun times.

And that brings me to the last point: Who are you people?

Seriously, if all of you are having severe problems integrating the ORM into your projects, why haven't I read about those in our issue tracker? Why haven't I seen a PR from you attempting to create the extension point that you need? Why haven't we had a single discussion about an actual issue that you were facing?

This is an open source project and everyone is invited to participate in evolving it.

[ostrolucky]

You are complaining about overall state instead of specific use cases. If you are asking to remove all final and make all private public, that will obviously never happen. You should create a discussion for each case so we can discuss constructively for some change that is actually doable. For example, you started explaining that you had a case of extending Query class. That was interesting, but you never explained why you needed that. I also had some use case for this that I had to workaround, so I would be curious what was your case. Once we know that, we can then talk about this and possibly open Query for extension. But please don't combine multiple use cases in one topic, as it gets difficult to manage and topic can't be closed even if some of the cases are valid.

[guilhermeblanco]

I'm late to the party, but I heard about this request... 11 years ago. https://mwop.net/blog/2012-06-28-oop-visibility.html
And again the response is the same, composition over inheritance.

[mgamadeus]

Thanks for your replies, the spirit here really seems to live up to the repo name.
We'll rework our framework and integrate Eloquent instead.

[derrabus]

Very well. What saddens me a bit is that you said that you've wanted to start a constructive discussion, yet you did not care to answer to any of the responses that you've received. Instead, you're leaving the "discussion" you were so much "looking forward to" with a poor attempt of insulting the people who took the time to reply.