-
Notifications
You must be signed in to change notification settings - Fork 165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Some reviews #1
base: master
Are you sure you want to change the base?
Some reviews #1
Conversation
@@ -11,10 +11,14 @@ | |||
public class Main { | |||
public static void main(String[] args) { | |||
// Setup | |||
|
|||
// Todo: tu verras dans la vidéo je ne suffix pas par Adapter car je range les adapters dans un package dédié du coup ici | |||
// todo: j'aurai eu un truc du genre SimpleUserRepository ou moi je nomme InMemoryUserRepository (en fait ma classe porte le nom de l'implémentation) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je pense que tu as raison, c'est plus clair de faire InMemoryUserRepository
pour l'implementation de UserRepository
(et aussi HazelcastUserRepository
)
var userRepository = new UserRepositorySimpleAdapter(); | ||
var idGenerator = new JugAdapter(); | ||
// todo : ici SHA256PasswordEncoder (=> je ne me pose pas de question de quelle est l'implem derrière ce qui n'est pas génant pour les adapters) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
je vais changer ca aussi
@@ -17,6 +17,7 @@ | |||
|
|||
@Bean | |||
public UserRepository userRepository() { | |||
// todo : essai d'injecter le in memory en pur java, ca devrait marcher aussi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment ca? La je suis dans l'app Spring, donc je me sers de Spring pour gerer mes dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Si en injectant les autres implémentations du même ports ici théoriquement ton appli devrait toujours fonctionner
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oue ok je vois ce que tu veux dire. En fait j'utilise une implementation ici et l'autre implementation dans Vertx. Si j'injecte les deux ici, spring va utiliser la premiere qu'il trouve ou alors il faut preciser quel implementation utiliser. Je pense que j'expliquerais ca dans le post plutot que te surcharger le code
@@ -16,6 +16,8 @@ private User(final String id, final String email, final String password, final S | |||
this.firstName = firstName; | |||
} | |||
|
|||
// todo : convention de java d'avoir le builder dans la classe ? | |||
// todo : perso je sors les builders pour les mettre à côté des use cases (c'est totalement arbitraire) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oui en Java on a les builder dans le pojo en general. Enfin d'habitude j'utilise Lombok https://www.projectlombok.org/features/Builder mais c'est pas encore compatible avec Java 10, du coup je l'ai fait a la main
@@ -1,3 +1,4 @@ | |||
// todo : spi ?? une abréviation de java ? perso je range dans un package qui porte le nom du types du port (ex. Loader / Repository / Gateway / Encoder ...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SPI = Serial Peripheral Interface Bus https://en.wikipedia.org/wiki/Serial_Peripheral_Interface_Bus
J'avais trouve ca une fois quand je m'interessais a l'hexagonal architecture. J'aime bien car ca represente bien ce qu'est la classe, une interface/port a implementer. Apres je t'avoue ne pas l'avoir vu souvent. Je voulais eviter de creer un package par type comme tu le preconises, car dans mon example je risque d'avoir une classe par package. Enfin c'est tjrs le meme probleme de vouloir faire un example simple, mais proche de la realite, mais si c'est trop proche de la realite, ca devient plus compliquer a comprendre
@@ -1,3 +1,4 @@ | |||
// todo : je rangerai la validation dans le répertoire use cases dans un sous répertoire validators par exemple |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, pas con, ca rendra les packages plus clair
@@ -8,6 +8,9 @@ | |||
import java.util.Optional; | |||
import java.util.stream.Collectors; | |||
|
|||
// todo : Pourquoi dans le in memory simple tu as nommé la classe UserRepositorySimpleAdapter et ici juste UserAdapter ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
J'ai rename 30 fois les classes et les packages, c'est pour ca que un review etait necessaire ;)
je vais update le nom
@@ -8,6 +8,7 @@ | |||
import java.util.Map; | |||
import java.util.Optional; | |||
|
|||
// todo : gros :+1: pour le mode in memory en pur java (sans spring) je pense que la sur couche spring Hazel Cast est overkill pour ce genre d'utilisation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oui en fait, je veux prouver qu'il est facil de changer l'implemention. A la base, je voulais faire un exemple in memory java simple + in memory DB (h2 ou mongo). Mais Java 10 est pas encore pret pour ce genre de truc, H2/JPA/Hibernate est pas vraiment encore compatible, donc le seul exemple simple que j'ai trouve c'est d'utiliser hazelcast
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes bonne idée ;)
c'est un peu ce que je te propose dans le fichier application/spring-app/src/main/java/com/slalom/example/spring/config/Config.java
Super, les gros concepts sont là 👍 :
Je pense que le point qu'on peut améliorer c'est sur l'arborescence des répertoires. Je trouve qu'elle est trop orientée technique et non métier.
Du coup tu as des packages un peu partout, pour un petit truc j'ai eu un peu de mal à m'y retrouver (peut-être parce que je ne suis pas très java ?) mais quid sur des gros projets.
Ce que je propose (tu le verras dans ma vidéo), c'est de regrouper par grand ensemble métier ce qui peut s'apparenter à un bounded contexte en DDD, si tu connais (mais de très très loin) et dessous j'ai une arborescence qui est toujours la même (un peu plus technique).
Dans ton cas on pourrait avoir un truc du genre :
comme ça tout ce qui concerne les users est rangé au même endroit
Tu as super bien saisi dans tes use cases le découplage via les interfaces, je te propose d'aller un peu plus loin (peut-être un peu overkill pour juste une prez) et c'est plus du design.
C'est d'utiliser le Design Pattern Stratégie pour faire du CQS (command query separation), un exemple grossier du truc :
Je rajoute une sorte de classe d'orchestration où juste elle connait mon répo par exemple, et plein de petites classes de manipulation de la donnée qui gravite autour (comme ce que tu as déjà fait).
Du coup ca me permet d'injecter uniquement cette classe et non toutes les petites classes, c'est ultra scalable et évolutif.
The end, je pense avoir fait le tour ;)