SocialPass / socialpass

Hosting the next generation of events
https://registry.socialpass.io
Other
1 stars 0 forks source link

Refactor services into models #221

Closed crypto-rizzo closed 2 years ago

crypto-rizzo commented 2 years ago

Nicolas and I agreed we began to mix two style guides (fat models vs services) Following the more recent one, we should migrate service layers to model methods, properties, etc.

crypto-rizzo commented 2 years ago

I think an added part of this issue should be setting up some tests for the root folder.

An easy start is migrating all service-related tests to these tests, once they have been converted to obj methods

crypto-rizzo commented 2 years ago

I think services could (and probably should) still play a role in the future architecture. However, most of our current services occupy the space of something that probably fits better closer to the model (either as method, property or queryset filter)

Without too much time or discussion, I've been thinking about the below architecture

An OK rule of thumb I see is that if a function takes only a singular model as its argument, it probably belongs closer to that model. Otherwise, it's probably a service.

devjoaov commented 2 years ago

Hey guys @hubertokf @halfmoonui @crypto-rizzo. Hope you are well.

As I said in the daily, I have a few concerns about the approach that we will use now.

First of all, I moved the services functions that used the models (Ticket, Event and BlockchainOwnership) and left as a service funcion just functions that not access our database (moralis_get_fungible_assets and moralis_get_nonfungible_assets).

After reading this article about why not using services if we could use model methods, I agreed we should use a mix of both: services layers and model methods. I agreed because the most part of service layer, Django already does with his ORM and everything related to model consumption should be in the model.

The worries I have now is the code readability, maintainability and scalability. The models.py reached almost 950 lines after transform service functions as model methods, and we just have 8 models registered in models.py. I think after transforming services into model methods, we introduced a problem of readability. If we are going to increase the quantity of models, we should think about the models.py file (if we are going to split the models.py).

IMHO, the service layer has pros and cons.

  1. Pros The code is cleaner than fat models approach. The code is separeted by where it will be used (api_scanner has services regarding api_scanner, api_checkoutportal has services regarding api_checkoutportal). We have a new layern to business-related things
  2. Cons The cons is the code escapes Django architecture, as we already have a place to put these functions. (Django is OOP, and functions may also be running away from Django architecture)

And the fat models approach also have pros and cons.

  1. Pros Uses model methods to model model-related things and the powers that OOP gives us.
  2. Cons The code is not very readable or scalable and the models.py file turns heavy.

Another question I want to ask to you guys is: the business logic of our project is really connected to our models?

halfmoonui commented 2 years ago

@devjoaov Thanks for the feedback. I'll put down my opinions about this issue. Please note, this is highly subjective.

Again, this is highly subjective. Also just from previous experience, I generally tend to have dev-related opinions which are often not popular among other developers.

crypto-rizzo commented 2 years ago

I agree with @halfmoonui here - I'm a big fan of not having code split across various domains with various naming structures. I think having fat models faces you to confront most of your complexity head on, rather than being able to hide it through clever naming, files, etc. I personally wouldn't say a file is a concern until 10,000 LOC.

However, I do like splitting up apps (views, urls, NOT templates) by their domain. Rarely (if ever) do these files interact with each other, and it makes for a bit cleaner file structure