Feat / Create an expense #18
Conversation
engrave-fr
force-pushed
the
feat/do-an-expense
branch
from
ec69ef7 to
79ad7f2
Compare
|
|
||
| use App\Domain\Expense\Expense; | ||
|
|
||
| interface ExpenseRepositoryInterface |
There was a problem hiding this comment.
Est-ce vraiment utile d'avoir la notion d'interface dans le nom ? La définition en tant qu'interface suffit à mon avis. Cela permettrait dans les constructeurs de services d'avoir la notion de ExpenseRepository qui est suffisant.
Dans le DoctrineExpenseRepositoryle implements est à mon avis assez explicite.
Qu'en penses-tu ?
| class Expense | ||
| { | ||
| public function __construct( | ||
| readonly private Uuid $id, |
There was a problem hiding this comment.
est-ce nécessaire de mettre l'Uuid en paramètre ? On ne pourrait pas le set directement dans le corps du constructeur ? À moins qu'il y ait une utilité que j'oublie ^^
| @@ -0,0 +1,16 @@ | |||
| App\Domain\Expense\Expense: | |||
| type: entity | |||
| repositoryClass: App\Infrastructure\Expense\Repository\ExpenseRepository | |||
There was a problem hiding this comment.
| repositoryClass: App\Infrastructure\Expense\Repository\ExpenseRepository | |
| repositoryClass: App\Infrastructure\Expense\Repository\DoctrineExpenseRepository |
| @@ -15,7 +15,9 @@ | |||
| "symfony/dotenv": "7.0.*", | |||
| "symfony/flex": "^2", | |||
| "symfony/framework-bundle": "7.0.*", | |||
| "symfony/messenger": "7.0.*", | |||
There was a problem hiding this comment.
peut-être faire une PR à part avec toutes ces configs et ajouts de packages et la merger pour que les autres puissent partir de la branche main ?
| @@ -0,0 +1,8 @@ | |||
| <?php | |||
|
|
|||
| namespace App\Infrastructure\Shared\Bus\Command; | |||
There was a problem hiding this comment.
c'est vraiment dans Infrastructure ? A mon avis, les commandes/query (et leur handler) font partie de l'application et ne devraient pas avoir de dépendances externes
Si je ne dis pas de bêtises, ta classe CreateExpenseCommand devrait implémenter cette interface. Si c'est bien le cas, dans l'état actuel, Application a une dépendance à Infrastructure :)
Il y a d'ailleurs ce vendor qui est sympa en hook git pour vérifier qu'on respecte bien les règles de dépendances : https://github.com/qossmic/deptrac
No description provided.