Remove references to entity generation, replace by proper entity modelling #8893
Comments
|
OK, but let's discuss about this specifically for Symfony apps, not for Doctrine or software design in general. My question: if I remove all my setters in all my Doctrine entities, will the rest of Symfony components keep working perfectly as before? For example, the Form and Serializer components? Thanks! |
|
Form and Serializer components I don't ever use IMO getters/setters/public properties are necessary for RAD of CRUD applications. Most of the applications on the market are of this type. Personally I use this approach with combination of domain specific features. I don't see a problem to have normal getters/setters in conjunction with domain specific methods in my entities. However, I don't like strict DD design. It's very tedious to write and I wish others would stop forcing it on us. I also don't see how entity generator prevents users to "properly" model entites. If you guys don't like getter/setter generation, just remove that functionality. It's still useful for generating classes and properties. |
|
By the way, in the "Getting Started" guide of Doctrine itself you can read this:
See http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/tutorials/getting-started.html |
|
The "problem" with setters is that they make the Model (Entity) anemic (as explained by @Majkl578 ). they don't communicate any meaning, your models should "speak" the language of the application ( But newcomers tend to struggle with this as it requires a good understanding of the domain and how you interact with it. But most of them are introduced with the easy and Rapid/CRUD building approach of getters and setters. For CRUD it's good enough, but in a well designed application the Symfony framework is a detail you only care about at a later time (thus don't use CRUD, but DTO, Commands or something). Most of this information comes from Cleancode from Robert C. Martin (Uncle Bob). highly recommended read for everyone.
Can you give an example? I always try to use clear methods, but nothing forbids me from using a setter when there no better method available. https://github.com/park-manager/park-manager/blob/master/src/Component/User/Model/User.php#L155 |
|
It's really confusing. They are forcing people to not to use anemic and in the same time anemic is forced to people by their docs. |
Yes, I realized slightly after posting this issue that these components leverage the same antipattern as well, unrelated on Doctrine. It should change there as well, but I can't really talk on that end since I have barely any experience with those.
Please, no public properties, that takes us to PHP 4 age. There are better means.
I don't see a problem with having getters (if their existence makes sense - not all fields are to be exposed), they don't change behavior. Setters on the other hand are different story, that's what this is all about.
Please don't be confused, there is nothing to be confused about. We have to start somewhere, and it's here. This issue is about Symfony's documentation, Doctrine is going to work on the same issue as well (see cross-linked issue above). It should also be noted that ORM 2.6, to be released soon™, will still have EntityGenerator. It will be deprecated in 2.7 and removed in 3.0 next year. There is plenty of time to work on improving documentation on all sides of the barricade. |
|
@Majkl578 You are citing the twitter post without linking to it! |
Well, the post referred to, both above and in the Twitter conversation was Value Objects in Symfony Forms by @webmozart There is also his talk from Symfony Con Barcelona where he covers all this too. |
|
Update: in my original comment I said "we're not going to make this change" where "we" means "the Symfony Docs team". I've changed it to "I'm against making this change" ... to be clear that this is my personal opinion. I hope you understand my comment well: However, what we could do is to add a new article explaining how to use this "proper Doctrine modelling" in Symfony apps for those developers who want to use this. That would be a really helpful resource. |
|
IMHO Why do you want to remove this tool ? Making things more complicated than they already are ? |
|
@Levure honestly, how often do you reverse-engineer entities from an existing database? |
You're right, sorry, I confused issues - it was mentioned in Doctrine's issue, not here.
This makes me sad. How I understand this is that one person here likes messy code, therefore wants to close this issue down.
Not true. Especially since there is plenty of time to make anything in Symfony not break with clean code.
This is one-off operation and you mostly need only mapping. You don't need whole PHP class full of setters. |
|
I like that you want to REORGANIZE a principe. I realy think it is by good thinks. PS. I'm totally dont understand argument about IDE |
|
I'm glad this issue has been raised, but in my opinion info on this topic is also lacking in the Doctrine documentation. The ideas raised in this thread alone sound very semantic and are underdiscussed in the Symfony community (at least the part I see). If the Doctrine documentation on entities can be improved, the Symfony documentation can focus more on using those models in the associated components like Form. |
|
(moving my comment from #7460) A side note about "Anemic domain model". Martin Fowler said in 2003 that he does not like it. A number of recent articles suggest that an Anemic model may very well be superior to a rich domain model with regards to testing and "SOLID" principles.
It is worthwhile to read some of this. The main idea, as I see it:
How this applies to Symfony and Doctrine, I leave for others to discuss. |
|
In reply to articles @donquixote shared: This is a great example of what DDD is not about, have some of these authors actually read the books on DDD? Domain Driven Design is about an information Domain, at the hearth of the system (it even says so on the cover). Having a method named There is absolutely no Domain concept in the whole article. If calling a method on the I see no purpose in the whole article other then trying to convince me that a Ridge Domain Model is bad, but without a proper explanation. Also a pure function is a function that always gives the same result with the same arguments, it has nothing to do with a Model (DTO, Anemic nor Domain), a Model is about something that holds a state or value, that's whole freaking purpose of DDD, to Encapsulate a Domain state and make it's communications explicit. Not about pure functions. He could have simple explained that Anemic Models are not all bad, that are situations were they are considered acceptable (like CRUD with RAD) for small applications or game development (as there are technical limitations that make DDD unpractical in this situation).
ActiveRecord is already about a Persistence Model, you don't put a Persistence Model inside an Domain Model and then complain it doesn't work, the author could have figured this themselves. ActiveRecord is an Infrastructure detail that in the sense of DDD belongs to another layer then the Domain. Yes, you can use ActiveRecord for persistence but it requires a manual mapping process between the Domain Model and the ActiveRecord Models (which is why ORM is favored for DDD, btw). There is nothing wrong with using a Model that is considered Anemic, if DDD does not fit the design as there is no actual Domain or there are to many technical limitations. Or if the Application is absolutely to be short lived or starting from scratch with a proper Domain in-place doesn't require to much work. But please for the love of sanity don't call it a Domain Model! Fowler called it an Anemic Domain Model because it's badly designed and contradicts Domain Driven Design. It is an anti-pattern, but a simple Model that is Anemic is not, just don't call it a Domain Model, OK? 👍 Edit: I changed my wording a bit, It wasn't my intent to insult anyone; I was horrible shocked about some of the bold statements in the articles that completely missed there point. |
|
@sstok There is no need for insults. The main question discussed in these articles, or what I take away from it, is which of the following two options is preferable:
The articles I mentioned label these two options as RDM and ADM. You might want to disagree with that, fine. To me it seems quite consistent with the way Fowler used the term ADM in 2003. In both of these options you can make a stupid example and then point out how bad it is.
If you use "model" as a term for the class itself, the class that holds the data, then yeah fine. They call it anemic, if behavior and data live in separate classes/objects, but the entire collection of classes is still a model in this second meaning of the term. |
It was not targeted at your personally but the articles (and how I think about them) 👍 (I should have made this more clear, sorry). I agree with the points you make, nothing wrong with an Anemic Data (not Domain) Model. My only problem is with the way the linked articles try to explain there point, and how the authors failed to explain this (ActiveRecord? and then in the second example the author uses a Factory). I am open to discussion, and I can understand and agree to a difference of opinions, I believe something till someone can provide arguments to prove otherwise 👍 (The DDD books actually talk about situations where DDD is not a suitable, which is why I don't believe the authors have actually read the books on DDD and merely base there opinions on something they heard from somewhere, it's dangerous and leads to misinformation and flame wars (I actually used to believe ORM is inherently bad because someone once told me that 😑 )). But to be clear, it was not my intent to insult you (or your believes). |
Ok, then take my first sentence as defending those who are not here :) My own motivation is this:
I think it is advisable to have a lot of reusable package-like code, and only a thin application-specific layer on top of that, which does nothing more but combine the building blocks from package code. Especially with a CMS, if you plan to build various sites which are all similar-but-different, then you want most of your logic to live in domain-agnostic components, not in domain-specific code. As for object types, you have
If all your application-specific behavior is living in the classes that also hold your data, and that are instantiated per "entity", but you still want to compose this from reusable package-level components, then you will have to repeat the same composition for every new instance of a domain object. You could say they become "Frankenstein compositions" (I just made this up). Perhaps the entity instance is now encapsulated from the outside (not sure), but this does not buy you much. With ADM, you would still have the equivalent composition in your service-level objects. But I think composition on this level is a lot more pleasant than having it in every entity. |
|
@sstok expressions like --> |
|
Will do ❤️ Like I said, I have some serious issues with the contents of the linked articles, that's the reason for my excessive reaction, not to excuse my actions. I changed my reply a bit, explaining it wasn't my intent to insult anyone (but I can't find a respectful word to describe my disbelief 🤫 ). OK. I will shut-up now 🙇🏼♂️ 😃 (or at least be less aggressive or condescending). |
|
As the name of the antipattern implies, anemic model means that the model lacks something very important like blood lacks enough healthy red blood cells or hemoglobin which is always bad if you want to live properly without external services like hospitals. I like the name of the antipattern as it nicely explains itself in a metaphore. There are very rare situations where having only getters and setters should not be considered anemic because it is complete already and does not need to have more behavior than to exist and does not need to protect more invariants, it's just a data structure (something like simple Settings object for example). But this is inherent more to OOP itself than to DDD. It's an OOP antipattern and it's always bad. There is always a better approach even in those reusable packages. There are certainly situations where using a hack or a bad practice is a good tradeoff for solving someone else's bad decision but that still does not make it a good practice. In Symfony I use Commands and https://github.com/thephpleague/tactician-bundle. I bind these commands to Symfony Forms so the default PropertyPathDataMapper is mostly good enough. |
Some things are actually better if they are are bloodless and inanimate. It makes their behavior more predictable. E.g. you don't want your car keys to obtain life and jump out of your pocket.
An object with only getters and setters can be an indication of poor design. If that's your definition of anemic, then I think I agree with you. But should an entity object be able to load related / referenced entities from the database? I would argue an object that can do this has too much blood, not too little. A good middle ground might be to let an entity object be responsible for a limited amount of internal or object-level constraints / invariants, which do not require injected services (or very few of them, perhaps?). Whereas "external" behaviors and constraints, and transactions between entities, would have to be handled by external services. With this approach don't necessarily end up with entities which have only getters and setters. But you might want to call it anemic because it has fewer responsibilities. So, perhaps this is just a debate about terms, in which case I have to apologize for the distraction. |
Perhaps #7460 would have been a more suitable place for this side-track. I just came here because the other one was closed as duplicate. |
Actually, we now have the Message (or Messenger) Component 👍
Loading entities from a database (or persistent storage to be more exact) in the Domain layer is an Infrastructure detail, so this would violate the separation of layers. You can make an Entity hold other entities (Aggregate) keeping the information together as you can't for example have a InvoiceRow without an Invoice to hold them, if you create the object but assign the invoice later your InvoiceRow would be an an invalid state as it's invariant is violated (a row cannot exist without an invoice). Which is indeed against OOP principles in general. Yes, you can make the
Actually this is perfectly fine 😁 in fact it's Domain Driven Design (or to a degree) 🤪. Many people misbelief that a Domain Model (Entity/Value object) is about doing everything (my former self included), that's in fact not the case at all. A Domain Model (or Domain Object to be exact) is about encapsulating business knowledge (and process) into a model (in the broad sense), persistence and services do have there existence in-here but in the rightful place. Persistence is handling in Infrastructure (the Domain only defines an interface), while Services (depending on there purpose) reside in the Domain or Application layer. The Domain Services handle what cannot be done in a Entity/Value object because it doesn't belong there (like changing other entities), but is still strictly related to the business process. The Application Services are usually more leaning towards the interaction (UI) part and e-mail messages (to name something). Nothing personal, but if you haven't read any of the books on DDD (in particular the ones from Eric Evans), I suggest you do 👍 when I got started with DDD most of my knowledge and information came from articles on the web, which give a good impression and are still helpful for some unclear cases, but don't cover all the topics what DDD is about, including why it was invented in the first place, muddying the waters and leaving me with half broken implementations and unworkable models. An anemic domain model as Fowler explained is about Entities without any own responsibility, all the actual business logic is placed in Services only, robbing the Entities from responsibility they are able to handle with ease (like a correct status), which basically turns them into Data transfer objects that are persisted. I think most people misunderstand this and fear that a Domain Model is just to complex and that an Anemic model is easier to work with, or that they try to hard, putting everything in Entities. As I said before, an anemic data model is not bad perse, but it's not a Domain Model for sure.
No problem, it's good to have conversations like these 👍 (my attack of before was completely unnecessary! It wasn't the first time I reacted, like this (but hopefully the last), and I am trying to more respectful and calm; Heck I am the one who wrote the (initial) constructive criticism guide for contributors 😳 ). I hope this doesn't discourage you from having conversations like these 💚 |
:-D. Yes. The intent of the key is probably very different.
Exactly. It's an indicator. But there are situations where having classes with getters and setters only is OK because they are actually not fully-fledged objects. Their intent is just to carry information. But the domain objects with their behavior still must exist. If they don't then it is closer to procedural programming where you don't send messages between objects but manipulate with datastructures (= calling setters or setting public properties on "entities") by calling procedures (mostly by calling methods on so-called service objects).
I would agree.
Sort of. You don't need to put everything into entities. But the object should always be consistent and it should protect all it's invariants ie never get into an invalid state.
And although I like DDD I think we don't need to refer to it since these principles are inherent to OOP. |
This is valid for statically typed languages like Java in the link. In PHP you can't define property type, so it's better to generate those dumb setters just to ensure basic data integrity. But again, this is valid only to RAD. |
|
My previous comment would not be valid anymore: https://wiki.php.net/rfc/typed_properties_v2 |
|
@JarJak this RFC is awesome, but in Symfony/API Platform we already have everything needed to ensure type safety using PHPDoc annotations (or other sources of metadata, including Doctrine mappings): |
phpDoc is not a type safety, it's merely just a documentation/hint that is usually relied upon as there is no better way (same applies to unions for example). |
|
@Majkl578 of course a native solution would be way better, but please read the links I posted... |
|
Le mar. 12 juin 2018 à 13:29, Steffen Roßkamp <[email protected]> a
écrit :
I think it is important to encourage best practices, but never enforce
them. It's also important to explain why and when to use them and sometimes
even telling people when not to.
Many if not all human beeings strive for freedom and have a habit to
develop resistance against applied force, so trying to force something
could even have the opposite effect.
I fully agree.
Explain, encourage best practices, but not enforce (and so, not removing
entity génération).
My first experiences (when discovering Symfony) was via entity generation.
I now write my entities by hand.
Never forget that Symfony is known to be hard to learn.
Regards,
… |
|
Here's the news: software design is hard! Yes, it really is! Believe it or not, but making state cross your application layers via state mutations rather than explicit interactions makes your software even harder to reason about. |
|
Le 21/06/18 à 21:52, Marco Pivetta a écrit :
Here's the news: software design *is* hard! Yes, it really is!
Believe it or not, but making state cross your application layers via
state mutations rather than explicit interactions makes your software
even harder to reason about.
@Ocramius : You don't need to be rude.
We are here to exchange - with respect of other opinions - to build a
nice and friendly documentation for *everyone* : starters, intermediate
and experts.
Regards,
|
Probably confusing sarcasm with rudeness here.
Right, so steer everyone off the quick way to complexity. Unsubscribing from the thread BTW: feel free to leave the "marketing features" well exposed. |
|
Do we have code examples (entire project, or some snippet) for the "recommended way" of entity modeling? |
|
Try with https://github.com/ShittySoft/fwdays-2018-doctrine-tutorial/tree/feature/specification-testing?files=1 - that's the material of my current workshop. |
|
@Ocramius interesting. The User class is interesting:
The relation of persistence record vs runtime entity object is one of the main design challenges for entity modeling with PHP. Perhaps some of the above points are by design, and others are because this is a "stub project"? |
The "register" method does send an email, but only through a behavior object injected into the method itself. This behavior object is not stored in a property, it is only used in the operation itself. This is a special case because the method is a static factory. Which leads to the question: If we wanted to add a similar behavior in the "login()" operation, would we pass the behavior object as a parameter to the login() method, or would we keep it as a property? |
|
Besides this bit:
The rest is by design. This is for a full day of challenging (dangerous) assumptions in software design, where attendees slowly revolve to this kind of implementation. There are optional bits such as "logging a login" or "preventing brute-force", which are trivially implemented if the state is correctly handled ONLY by the aggregate root (the
The word
Built already a few applications with these approaches: they work fine, and it is very hard for new developers to misunderstand how to use the domain components. The words in the API match the ubiquitous language as much as possible too.
Only retain state that you are responsible for. The lifecycle of services and See also http://ocramius.github.io/blog/on-aggregates-and-external-context-interactions/ |
|
I'm afraid the example is simplified to a level where the questions from this thread are not really touched. Or again I might not be looking in the right place.
A good example would demonstrate how these steps are not really necessary, and the problems can be solved in other ways. Unless you actually agree with the above :) I am writing this because I think the current discussion might be too theoretical. I personally have implemented different domain model architectures for different purposes, and I have not found the ideal way yet. But most of the time, I follow the entity model of the CMS I work with (Drupal 7, Drupal 8), which you might like or dislike for various reasons. Some observations from this:
A lot can be said about this, and most of it would be out of scope for this issue. But I imagine one main reason for the really heavy entity objects in Drupal 8, and their dependency on services, was the implicit or explicit desire to make them not anemic. |
In "real world" apps, this is where the concepts of "read model" and "view model" come in. Aggregates are designed to keep all the state private, and never really tell anything to the outside, which gives you a degree of freedom in changing logic. Read models allow for aggressively optimising read operations via low-level operations (caching, memoising, SQL optimisations, async parallel execution) and for keeping the results on-point with what is expected. I don't expect people to split reads/writes from this discussion, but be aware that valid and battle-tested solutions do exist here. I'd already be very happy to get rid of the concept of "setters" here.
Terrible idea to bind entities to forms. https://webmozart.io/blog/2015/09/09/value-objects-in-symfony-forms/ instead. I personally don't use symfony forms, but all my forms work with arrays or DTOs these days.
We're probably going too deep into tactical DDD concepts, but:
Yes, Doctrine ORM is a mapper like any other! You should be able to write trivial reflection-based mappers for any kind of aggregate. The referenced repository includes some primitive implementations that are sufficient to save/load entities: https://github.com/ShittySoft/fwdays-2018-doctrine-tutorial/blob/feature/registration/src/Infrastructure/Authentication/Repository/FileUsers.php
Please don't: dependency injection is only applicable when lifecycles are compatible. This is why it is usually only valid for long-lived instances (services).
I was asked for a clean code example: well here it is. I think this is the way to write software in the PHP language and Doctrine ORM (at the best of my current knowledge, as of today, 2018-06-22). I think this quote is strictly related and very much fitting the thread.
As for the Drupal bit: I can't comment, as I don't remotely know how Drupal works, and I strive to stay independent from the tooling in order to keep the language close to the business and the upgrades or migrations manageable. My tip: it shouldn't matter if it is in the context of Drupal or not. |
|
If I can give a position about this issue, here's what I'm thinking about. Should Symfony have a position about model design?
If we look at the main goal of Symfony, the main goal is to manage HTTP requests/response, what we do with this requests/response and all the logic which is involved between the request and response is up to us. So, should Symfony know about our lower-level policies?
Symfony isn't designed for this, even if the Security component use a I know that the debate is about rich vs anemic models but well, should Symfony care about this?
Symfony can easily work without a single model and you know what ? That's probably the best idea we can have, our business logic isn't tied to Symfony, we use Symfony in order to have a "set of tools" that help us to build modern, reliable and evolutive applications, no more. I'm fully committed to the vision of @Ocramius when it comes to the Form component, using DTO or value object is probably the best idea we can have, why ? Simply because depending on a Symfony component is bad, why ? Simply because the framework can evolve and the component can change, what if some functionality disappear and our logic no longer work? Same things go when it comes to API-Platform and what @dunglas has designed, we can use the internal library but what if we need to design our own logic and adapt API-Platform to our lower-level policies? Well, Dunglas and the core team do a pretty solid work and we can easily do it, we're not tied to his main vision when it comes to handling operations and other internal logic. For me, Symfony shouldn't give a "sterile" and "final" vision, the developer should be in control of what he do when it comes to lower-level policies, the developer should be able to adapt his logic to the user need and no matter what it does with it, not a single library should say :
So, what about the idea of bringing a rich model to the documentation ? I'm not fully okay for it, Symfony could easily say :
Symfony doesn't care about our model logic/design and it shouldn't IMO but well, like said before in this thread, the final choice come to the developer, not Symfony. |
It sounds interesting, but am not sure I am following. But perhaps I got this all wrong? Perhaps a DTO in your sense is not even specific to an entity type, and does not do any validation by itself? E.g. an I could read about "read model" and "write model" and "DTO", but this does not guarantee that I get the same idea about it as you do.
Again I am not sure I understand what you mean. Of course not every form maps 1:1 to an entity type, but for sure such forms do exist and are quite common. The entity object itself should be agnostic about forms.
This is interesting. The solution uses reflection to access private properties, so you don't need any getters. This looks like a hack to me. I am open to unorthodox solutions, but I would need some convincing why this is a good idea, e.g. why this is better than public properties. I guess the idea is that only some selected and trusted classes are supposed to access those private properties. So you are emulating something like a "friend class" mechanic, except that without a native "friend class" feature, it is not documented which classes are allowed to access the private properties. You are then using serialize/unserialize to store the object to disk, which again frees you of the need for setters and getters. I don't really like serialized objects, I think that the persistence model should be independent of the runtime model. Perhaps I am wrong.
I am not talking about dependency injection containers here, just about injecting services into a component which uses them, instead of letting this component access the global container. We can assume the life cycle of an entity object to be shorter than that of a service. A service should not get an entity injected, but an entity can be injected with a service on construction time. Or, which is perhaps better, avoid having entities depend on services. There are practical reasons why Drupal uses the global The advantage is that the entity does not carry services around. The disadvantage is that testing the entity requires a global
Yes, this is what I am looking for. An example that eliminates the common reasons why people add getters and setters to their models. I am not yet convinced that your example sufficiently shows this, but we may be getting there. |
|
TIL answering to a thread re-subscribes you to it. Gonna try to keep this short:
A read model is an abstraction on top of a read-only data store. It could be a class containing an SQL query, for example.
Usually
Simple: don't let forms talk to entities directly, as you are mutating state without checking business invariants. Just because a field called
Mutating a blogpost is not an atomic operation. You can:
Some of these can succeed, some of these can fail depending on what you enforce as "valid" inside your concept of BlogPost (which is a terrible example BTW, because it's the typical developer over-simplification of business domains). The above ones are either 7 different forms or 7 different interactions to be piped in a queue of operations to be executed all at once (UX decision). Don't represent state in the UI and then slam it back into the entities à la foie gras, because you end up duplicating biz constraints everywhere. The single "migrate the URI" scenario is already a nightmare once anemic models are in play. Also, do all other scenarios lead to an immediately published post?
No, it doesn't - it is told what should happen once the interaction is translated from a form into a method call (
The public function purchaseAction()
{
$this->securityCheck->assertValidUser($this->request);
if (! $this->form->isValid($this->request)) {
return new ViewModel(['form' => $this->form]);
}
$data = $this->form->getData();
$user = $this->authentication->requireUser();
$product = $this->products->get($data['product']);
// actual domain logic call. 'amount' was already validated
$product->purchaseWithUser($user, $data['amount']);
$this->notifications->send($user, 'Purchase completed');
return new ViewModel(['success' => true]);
}
It doesn't matter if you serialize it, json-encode it, gRPC it, transform it to YAML and back (good luck) and then finally push it through memcached served through MySQL. Mapping is not an OO concept, but an FP and procedural concept (when persistence is involved).
Just use reflection there.
Yes, it's an example aimed at showing that persistence can be easily achieved, and that "thinking in tables" is easily avoided. People should use whichever persistence layer fits their needs best, and not take Doctrine ORM and its semantics for granted.
About the |
|
Thank you for this issue. |
|
Seems like we've removed all mentions of |
and others
Hello,
there are lots of users using
doctrine:generate:entitiesnowadays. This must stop.Symfony should provide its large user base decent article on designing/modelling entities in a proper way, with business methods and as few setters as possible, not by generating them from database. Such generated entities lead to a tediously known anemic domain model, something that does more harm than good: breaks encapsulation and completely undermines the purpose of ORM. It'd probably be even better to not use ORM at all and use DBAL directly, the result would be very similar without runtime overhead.
Note that Doctrine 3.0 will remove support for code generation entirely, so the better we educate the users, the better.
This issue is a direct response to discussion in doctrine/DoctrineBundle#729.
Thanks.
The text was updated successfully, but these errors were encountered: