Open olivierobert opened 3 years ago
Hi olivierobert,
Thanks for your feedback.
For me, the controller does two main jobs, validating the input, and serializing the output. All logic will be done in service. This is easy when I call UserService::..
for debugging and reusing the service in multiple places. In the service, there will be no need to care about input or output. The service will handling logic and working with the database.
In the model, for example model Keyword, the method total_result_time_text
I can consider it to be an attribute, and will be reused many times, and when needed, just edit total_result_time_text
, everything will change.
If this is not good, I would be very grateful if you can give me some advice.
Extracting code to small objects is generally good. Keeping controllers thin is also good. However, services should not be used for the goto object to extract code :-)
Presenters are a better option to extract code that is only used in view templates. This article shows the difference between presenters and services: https://www.rubyguides.com/2019/09/rails-patterns-presenter-service/
Public methods in the model for formatting data can use decorators as they do not alter the model behavior. Here is an article on how to use it with SimpleDelagator: https://medium.com/@kosovacsedad/ruby-on-rails-decorator-design-pattern-b54a1afd03c8.
It's interesting and wonderful to hear about presenters
and decorators
. Thanks for your new knowledge.
Extracting functionality to classes is key to stick to SOLID principles. The codebase has lots of small classes which is good. However, using the right pattern for extraction is also important. There are classes in the codebase which could be extracted as a different type of classes such as decorators and presenters.
For instance,
ActiveMenuService
is not really a service as its main use is in view templates.Similarly, the model
Keyword
has mainly public methods again used in view templates:https://github.com/tamle-dev/google-crawler/blob/e96384b0aa9a5473f7206c7f5c16aab5177f002e/app/models/keyword.rb#L18-L32
Eventually, all classes in
UserService
are not actual services and are closer to non-Active Record models.What do you think about this?