Skip to content

Conversation

NguyenDa18
Copy link
Contributor

Inspired by Brad's design in his NodeJS API Masterclass with Express & MongoDB.

@bushblade
Copy link
Collaborator

Hi @NguyenDa18 looks good to me but I'm wary of changing code from course code that doesn't need fixing.
The code as is works just fine and has no issue. I think unless it's solving some issue in the Udemy course then the code should probably stay closer to what the students see in the course?

@NguyenDa18
Copy link
Contributor Author

@bushblade I thought that was what the originalcoursecode was for, and there are some examples of refactoring in the master branch that make the app better -this change also demonstrates good practices that Brad used in his Node API class.

@bushblade
Copy link
Collaborator

True but all of the refactors so far have solved issues or bugs.
If we refactor to represent the node course then how far do we go?
Should we implement separate controllers? Error handling middle ware?
I'm not sure.
I think the code still needs to be kept reasonably close to the course.
I know Brad is planning on updating the course at some point so I'm sure he would adopt the architecture from the node course then.
I won't close though as I don't feel I can make a solid decision here, perhaps it should be left to Brad to decide?

@NguyenDa18
Copy link
Contributor Author

@bushblade Sounds good. Those suggestions sound good as well -implementing separate controllers and middleware like we do in the Node API course. You can just keep the PR open -thanks

@bushblade
Copy link
Collaborator

No worries.

@bushblade bushblade added enhancement New feature or request stale labels Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants