Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Some reviews #1

wants to merge 1 commit into from

Conversation

mickaelw
Copy link
Collaborator

@mickaelw mickaelw commented Jun 5, 2018

Super, les gros concepts sont là 👍 :

  • Ports/Adapters = injection de dépendance (découplage entre le coeur/core et le monde extérieur)
  • Use case
  • Domain

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 :

/application
/configuration (la config global du projet)
/user
   /adapters
     /primaries (on peut résumer à : ce qui afficher la donnée + action user)
       /controller
     /secondaries (on peut résumer à : ce qui manipule la donnée)
       /encoders
       /id-generators
       /repositories
      (des fois je re fais le distinguo entre real et in memory)
   /configuration (si tu as des trucs dédiés genre des routes API ou la DI pour cette ensemble)
   /domain
      /entities
      /exceptions
      /ports
      ...
   /usecases
     /validators

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 :

interface Query {
	List<User> handle(UserRepository userRepository);
	Optional<User> handle(UserRepository userRepository);
}

interface Command {
	void handle(UserRepository userRepository);
}

class UserHandler {
	private final UserRepository repository;
	public UserHandler(final UserRepository repository) {
		this.repository = repository;
	}
	List<User> query(Query query) {
		return query.handle(this.repository);
	}
	Optional<User> query(Query query) {
		return query.handle(this.repository);
	}
	void command(Command command) {
		command.handle(this.repository);
	}
}

class FindUserById implements Query {
	private String userId;
	public FindUserById(String userId) {
		this.userId = userId;
	}
	@Override
	public List<Optional<User>> handle(UserRepository userRepository) {
		return userRepository.findById(this.userId);
	}
}

// dans le code
new UserHandler().query(new FindUserById("1"));

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 ;)

@@ -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)
Copy link
Owner

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)
Copy link
Owner

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
Copy link
Owner

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

Copy link
Collaborator Author

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

Copy link
Owner

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)
Copy link
Owner

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 ...)
Copy link
Owner

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
Copy link
Owner

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 ;)
Copy link
Owner

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
Copy link
Owner

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

Copy link
Collaborator Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants