-
Notifications
You must be signed in to change notification settings - Fork 27
Space- Yieni & Shonda #4
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
base: master
Are you sure you want to change the base?
Conversation
…al, customer. relate rentals to customer and video
…h their tests for video and customer
…w on video controller, created customer yaml
… and check out. updated rental validation for numercality. messed up rental controller tests
Co-authored-by: yieknee <[email protected]>
Video Store APIMajor Learning Goals/Code Review
Functional Requirements
Overall Feedback
Additional FeedbackGreat work! Y'all built a fully functional API! Code Style Bonus AwardsWas the code particularly impressive in code style for any of these reasons (or more...?)
|
jmaddox19
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
| @@ -0,0 +1,34 @@ | |||
| class Rental < ApplicationRecord | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both of the custom methods here are so well-written!
| self.available_inventory = video.available_inventory | ||
| self.videos_checked_out_count = customer.videos_checked_out_count |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The redundancy of putting these two fields on rental to mirror the fields on video and customer introduces the risk that a bug could get introduced to cause them to go out of sync. If they did go out of sync, how should the system respond to that? Which number should it trust?
Because of that, it's generally best practice not to have redundant values intended to track the same information.
Assignment Submission: Video Store API
Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.
If you didn't get to the functionality the question is asking about, reply with what you would have done if you had completed it.
Reflection
/customers&/videosendpoints? What does the time complexity depend on? Explain your reasoning.POST /rentals/check-inendpoint? What does the time complexity depend on? Explain your reasoning.