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

Feature/recruiter crud refactor example #42

Closed
wants to merge 2 commits into from

Conversation

jordimorillo
Copy link
Collaborator

This is a little example about how to bring some value to the models by extracting the logic from the controllers.

Copy link
Contributor

Choose a reason for hiding this comment

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

This exception is much better than the one I made in recruiters, you throw the message and the response code in the same constructor.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

By extracting responsibilities in separated files you make the code, for your team mates and yourself:

  • easier to understand
  • easier to maintain
  • easier to escale

Code minimalistic every time you can and grow the code horizontally, not vertically.

Copy link
Contributor

Choose a reason for hiding this comment

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

Now I understand.

Instead of making the code extend downwards and becoming very long and difficult to maintain, smaller, more modular components are created.

Copy link
Contributor

Choose a reason for hiding this comment

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

  • correct separation of logic to the controller
  • In the catch block, you pass as a parameter an instance of the emptyListAdmin exception, inside the block you use HttpResponseExeption to throw the exception and to execute the code.

Copy link
Contributor

@pupadevs pupadevs left a comment

Choose a reason for hiding this comment

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

  • I found it all very correct and efficient the way you approached this little implementation to separate the logic from the AdminController, I have left simple comments in parts of the code. You have helped me to see how to deal with exceptions in a more optimal way.

The only thing I see wrong is the branch name, but otherwise everything is fine. 😸

I approve this refactoring 👍🏼

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.

2 participants