Composite identities formed with identity of parent - good or bad practice?
They are a good practice when they are used properly: when the domain expert identifies things locally (eg "the John from Marketing") they are correct, while they are wrong otherwise.
In general, whenever the code follows the expert's language, it's correct.
Sometimes you face a globally identified entity (like "John Smith") identified locally by the expert when he talks about a specific bounded context. In these cases, BC requirements win.
Note that this means that you will need a domain service to map identifiers between BCs, otherwise, all you need are shared identifiers.
References to internal entities - transient or long term?
If the aggregate root (in your case Product
) requires the child entities to ensure business invariants, the references must be "long term", at least until the invariants must hold.
Moreover you correctly grasped the rationale behind internal entities: they are entities if the expert identifies them, mutability is a programming concern (and immutability is always safer). You can have immutable entities, either local to other or not, but what make them entities is the fact that the expert identifies them, not their immutability.
Value object are immutable just because they have no identity, not the other way!
But when you say:
However the need to store this reference is for reporting and searching purposes only
I would suggest you to use direct SQL queries (or queryobject with DTOs, or anything you can have for cheap) instead of domain objects. Reports and search don't mutate entities's state, so you don't need to preserve invariants. That's the main rationale of CQRS, that simply means: "use the domain model only when you have to ensure business invariants! Use WTF you like for components that just need to read!"
Extra notes
When querying the product for associated features, the Feature object is returned but the C# internal keyword is used to hide any methods that could mutate the entity...
Access modifiers to handle modifiers in this context are a cheap approach if you don't need to unit test clients, but if you need to test the client code (or to introduce AOP interceptor, or anything else) plain old interfaces are a better solution.
Someone will tell you that you are using "needless abstraction", but using a language keyword (interface
) does not means introducing abstractions at all!
I'm not entirely sure that they really understand what an abstraction is, so much that they confuse the tools (a few language keywords that are common in OO) for the act of abstracting.
The abstractions reside in the programmer mind (and in the expert's mind, in DDD), the code just expresses them through the constructs provided by the language you use.
Are sealed
classes concrete? Are struct
s concrete? NO!!!
You can't throw them to hurt incompetent programmers!
They are as much abstract as the interface
s or abstract
classes.
An abstraction is needless (worse, it's dangerous!) if it makes the code's prose unreadable, hard to follow, and so on. But, believe me, it can be coded as a sealed class
!
... and thus ensure the entity is immutable to the calling application service (in a different assembly from domain code).
IMHO, you should also consider that if the "apparently immutable" local entities returned by the aggregate can, actually, change part of their state, the clients that received them won't be able to know that such change occurred.
To me, I solve this issue by returning (and also using internally) local entities that are actually immutable, forcing clients to only hold a reference to the aggregate root (aka the main entity) and subscribe events on it.