Skip to content

Conversation

@aminx4
Copy link

@aminx4 aminx4 commented Apr 14, 2016

Proposition for an hexagonal architecture implementation with a managerEvent service and MongoDB persistence.

@thebignet
Copy link
Contributor

Ce qui aurait été pas mal, ça aurait été d'aller un cran plus loin et de faire un module gradle par adapter, donc un pour l'adapter MongoDB, un pour la partie métier. L'intérêt de cette solution, c'est de mieux gérer les classes et dépendances.

La module mongo dépends du module domain (et pas inversement). Les repositories sont déclarés dans la partie domain et leur implémentation dans la partie mongo. Impossible alors d'utiliser des dépendances mongo dans le module domain.


apply plugin: 'java'

sourceCompatibility = 1.8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pourquoi avoir enlevé cette ligne ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'ai enlevé car mon IDE m'a indiqué que la variable en question n'était pas utilisé

@ldez
Copy link
Contributor

ldez commented Apr 14, 2016

Globalement :

  • dao -> persistence
  • les interfaces avec un I majuscule (nous ne sommes pas en C#) sont généralement un signe de défaut de conception.
  • faire une implémentation avec Mongo à ce state du projet ne me semble pas pertinent.
  • utilisation de Mock ne me semble pas pertinent.
  • utilisation de la branche master au lieu d'une branche dédié n'est pas une bonne pratique dans le contexte.
  • il est préférable de créer une issue avant de faire une PR afin que nous puissions en discuter avant.

public class MangoEventDao implements IEventDAO {

public static final String EVENT_DB = "EventDB";
public static final String MONGODB_LOCALHOST_27017 = "mongodb://localhost:27017";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l'url écrit en dur ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mauvais nommage de la constante

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Écrit en dur car il n ya pas de fichier de conf pour l'instant.

Tu proposes quoi comme nommage ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when you will have a conf file you probably won't expose it to you application anyway, I'll abstract it into a config class. So you can put your hardcoded url into a config class right now, it will be easy to evolve configuration system later.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean that i create a ApplicationConfig.java that contains constants of my mongodb server parameters ? then i call this class in my dao ? i will create a dependency between my DAO and my ApplicationConfig ? Do you have any suggestions please ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You got the idea I guess. If ApplicationConfig is too broad a dependency, you can have a MongoConfig.

@mbaechler
Copy link
Contributor

First of all, it seems that people are shy and then they didn't congratulate you @aminx4. Thank you for your PR and for moving this thing forward.
It goes in the right direction but I agree with @thebignet : you we want to prevent the domain from knowing about technical details, the first thing is to isolate it into a distinct module.
Can you try to do this ?

@aminx4
Copy link
Author

aminx4 commented Apr 15, 2016

Thank you. I will try. But i think that i need more details to do it perfectly. Thank's for your help.


@Test
public void shouldPersistInDataBase() throws Exception {
MongoDatabase mongoDatabase = MangoEventDao.getDataBase();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't really test anything related to MongoEventDao, do you ?

Copy link
Author

@aminx4 aminx4 Apr 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oupps !! Sorry ! i test that i can insert something in db but not MongoEventDao. i will do it.

PlannerRepository.java : Delete the index id append
PlannerRepository.java : rename persist to save
PlannerRepository.java : delete empty lignes
FacebookAdapter.java : Delete the class
MongoConfiguration.java : Create a mongoConfiguration class for externelize database parameters
MongoEventDao.java : change the signature of the constructor to get a MongoDatabase
MongoEventDao.java : Delete empty lines
MongoLauncher.java : create a MongoLuancher class
EventDAO.java : rename an EventDAO from IEventDAO
EventDAO.java : rename persist to save add find and findByKey methods
Delete ApplicationConfig.java
EventManagerTest.java : Delete static import of AssertJ and remplace it with Junit Asset
Create FongoLauncher for portable mongodb
PlannerRepositoryTest : rename method test
Add FakeMongo dependency and changing naming format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants