1 month ago

Constructive feedback request -- Controller Method Optimization

Posted 1 month ago by jcgivens21

Hey all, as I'm on this laravel journey, right now I'm working on refactoring code. I'm converting my controllers to a more CRUD style design, and it's making everything look a little nicer; however, I am wondering what the next step is. There's a lot written here, so thanks in advance if you decide to help. Here is a store method for my "EmployeesController," and I was hoping I could get some feedback on improvements to this code:


  1. $this->validate: Should I move this validation logic into a Request class? Or is this generally overkill?

  2. I'm creating the Employee here and saving it directly in the controller. Is it better to decouple this logic? I feel I've "slimmed" my controller based on the number of methods, but there's still a huge amount of text here, and I'm not sure if this is the best place to handle this Employee saving logic.

  3. In line with #2, there is some condition-based logic that when the user is inputting the employee data, they select whether the employee is paid Hourly or by Salary, and then I accept either of those values from the form, and I also calculate the other value even if it isn't sent and store both in the database table, in addition to whether it "is_salary" so I know when displaying the employee whether or not to display the values as hourly or salary. Condition-based logic like this is best in this controller logic?

  4. Also with #3, there is some calculations being done in addition to the condition-based logic. Should calculation results be stored in the model/database tables or should those be handled real-time? It was simpler when I started to just store everything in the table without worrying about calculating these values upon request--but in proper design, should only 1 of those values be saved to the database and the other always calculated?

  5. Once the employee is added, I have a Toastr notification (basically a popup) that informs the user that their form submission was successful. I do this in basically EVERY function in the program, and in my Layouts.app blade template, it just picks up these Toastr notifications. I feel this is a very good implementation, and it works well for me, but I'm open to criticisms of this.

  6. On the employee creation submit form, there are 2 submit buttons. One button (which is labeled "Submit" is just to submit the employee and then redirect the user back to the page that displays all employees. The second button is labeled "Submit & Return To This Page" and its function is to return the user back to the same form so that they don't have to click an additional button to get back here to add more employees. In the controller, I just check which button was submitted and redirect based on which button was pressed. Any thoughts on this setup?

  7. Other general thoughts?

If you've made it this far, thank you for at least reading. I look forward to the feedback.

Please sign in or create an account to participate in this conversation.