In my new job, I'm getting a hard time understanding how they want to model things... they are using Domain Driven Design. For example, I come across this kind of code:
$userRepo = new UserRepository($this->userModel);
$virtualClassRepo = VirtualClassRepository::fromModels($this->virtualClassModel);
$user = $userRepo->getUserById(Id::fromText($command->userID));
$virtualClass = $virtualClassRepo->retrieveClassByID(Id::fromText($command->classID));
$rating = $user->watch($virtualClass)->rate($command->liked);
$userRepo->storeRating($rating);
Where the watch()
function returns this:
public function watch(VirtualClass $class): UserVirtual
{
return new UserVirtual(
$this->ID(),
$class->id(),
);
}
To me this looks like super unnecessary, so many classes when this could be so direct, but I might not be getting the point of it...
Like I would just call a model function rateClass($userId, classId, $rateValue)
Is there a benefit in the original code? I did hear something about "explicit state modeling" but I'm not familiar with it.
The other thing is the abuse of static methods to create classes, and hiding the constructors, and I found situations where the class instantiated is meant to be saved, and other where the another same class instance (created from another static method) is not suitable for storing. This is driving me crazy.
This is what I sent to my lead (context: the class cannot be scheduled, i.e. added to a user, if the class was not published, and the same class is used to represent an already existing instance, in the delete()
method of the controller). These sorts of validations would normally be done in a preSave()
method in other frameworks.
If you create FromData you check that the class is published.
If you create FromId you don't, because you don't have class data there. You only have the IDs of the db record.
If you were to need the class data, it would be unnecessary from the delete() method since why would you need class data?
So the whole thing looks weird```
-
2I suggest you wait for the lead dev's answer (strange to talk about these very important things asynchronously by the way) and ask separate questions on SE for each of your points if their answer doesn't dissipate your doubts.guillaume31– guillaume312023年10月26日 07:29:50 +00:00Commented Oct 26, 2023 at 7:29
-
user, ok. userRepo, ok, virtualClass? virtualClassRepo? watch? can you afd some context here to explain what the virtualClass stuff is in relation to the overall function (which i assume is a user submitting a rating for a store)Ewan– Ewan2023年10月26日 14:15:50 +00:00Commented Oct 26, 2023 at 14:15
1 Answer 1
Though tempting, it wouldn't be fair to call cargo cult based on the little amount of code you shared.
DDD is all about a ubiquitous language shared by domain experts and developers. The terms of the language spoken by the experts have to be reflected in the code. If a UserVirtual
is some kind of participant in a virtual class that can do a range of things that a normal user cannot do, then it makes sense as a separate class. It's not "so many classes", it's "just the right amount of classes to encode meaningful domain concepts".
What's for sure on the other hand is that putting a dev in front of a code base without first explaining him/her the coding style and architecture/design philosophy is... dubious.