cloud-erp-school-system / backend

GNU General Public License v3.0
0 stars 0 forks source link

Restructured #25

Closed Aurrix closed 3 years ago

Aurrix commented 3 years ago

As promised just structural changes. I will try to write migrations and entities in the next pull

patricksan commented 3 years ago

I would prefer that we don't go in the direction of package client.child and then a separation per module. The idea of the project is to have a vast number of modules. With this approach the amount of packages will increase substation.

Also, with hierarchy we can easily lose the ability of using protected method.

I'm ok with using a super package for the big module, like client, but I think inside it it should come direct:

What do you think? Could you reconsidere that, please?

patricksan commented 3 years ago

@Aurrix could you also check the SonarCloud Code Analysis, please? It failed for this PR.

Aurrix commented 3 years ago

I would prefer that we don't go in the direction of package client.child and then a separation per module. The idea of the project is to have a vast number of modules. With this approach the amount of packages will increase substation.

Also, with hierarchy we can easily lose the ability of using protected method.

I'm ok with using a super package for the big module, like client, but I think inside it it should come direct:

  • controller

    • dto (the idea here is that dto is just used for controller)
  • model

    • hibernate classes
  • exceptions
  • repository
  • service

What do you think? Could you reconsidere that, please?

@patricksan I disagree here. I would suggest at the very least use package per entity strategy. I can move those packages to the same level as client if you don't like having child package. It will become more difficult to maintain project in the future as number of entities will grow. If we would have a microservice with just a couple of entities that would be fine but as I understood we will have a big fatty monolith. If you think that's wrong let me know why.

patricksan commented 3 years ago

@patricksan I disagree here. I would suggest at the very least use package per entity strategy. I can move those packages to the same level as client if you don't like having child package. It will become more difficult to maintain project in the future as number of entities will grow. If we would have a microservice with just a couple of entities that would be fine but as I understood we will have a big fatty monolith. If you think that's wrong let me know why.

I understand your point. It is also valid. But then I would go in this direction:

org.erp.school.controller.client
org.erp.school.controller.b
org.erp.school.controller.c

and not

org.erp.school.client.controller
org.erp.school.b.controller
org.erp.school.c.controller

This will avoid to have everywhere packages reposition, like controller, dto, models, service My biggest concern with this approach is with generic approaches. Like document is generic to all other entities.

My suggestion would be let's do with the approach I first described. When it becomes to have a lot of class we can review. I would go for this approach now, having 3 Controllers, 3 services and so one.

patricksan commented 3 years ago

Hey @Aurrix, could we do intermediate merge to master? Like this way we can avoid that others developer stay too far from what you are doing.

I also want to create some testing for Download API Controller.

sonarcloud[bot] commented 3 years ago

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 5 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

patricksan commented 3 years ago

Thanks @Aurrix! I merged to the master.