Karis & Dionisia VideoStoreAPI - Edges#20
Open
larachan15 wants to merge 45 commits intoAda-C10:masterfrom
Open
Karis & Dionisia VideoStoreAPI - Edges#20larachan15 wants to merge 45 commits intoAda-C10:masterfrom
larachan15 wants to merge 45 commits intoAda-C10:masterfrom
Conversation
created controllers and models
testing for customer model
created tests for movies model
added routes
customers controller and tests complete
movies controllers and tests passing
…ustomers and added tests for those validations
Karis setup
Karis setup
Karis setup
fixed avail inventory issue
merge conflict fixed
Dionisia work
rentals tests
rentals controller tests added
added tests for model methods
Video StoreWhat We're Looking For
|
| # binding.pry | ||
| if rental.save | ||
| rental.movie.increment_inventory! | ||
| rental.customer.decrement_movies_checked_out_count! |
There was a problem hiding this comment.
There's a lot of work being done in this function. Could you wrap this all up in a model method (maybe Rental.checkin(customer_id, movie_id)?
There was a problem hiding this comment.
Also, what happens if the rental saves, but incrementing the inventory or decrementing the checkout count fails somehow? You would be in a sticky situation.
We didn't talk about this in class, but the solution here is to use a transaction. If you're interested in learning about this, see https://api.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html
| # binding.pry | ||
| movie.decrement_inventory! | ||
| customer.increment_movies_checked_out_count! | ||
| # binding.pry |
There was a problem hiding this comment.
As above, it would be great to wrap all this up in a model method.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Video Store API
Congratulations! You're submitting your assignment!
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.
Comprehension Questions
First, what is
n? The number of movies or customers? You should specify this explicitly.Second, what about finding the rental count? In your code you track this count in the database, so it doesn't add any extra time to the
indexoperation. Are there other ways to do it that would?POST /rentals/check-inendpoint? What does the time complexity depend on? Explain your reasoning.nis the number of Rentals, this is not quite correct. Calling.ordersorts the entire list, which means the time complexity isO(n log(n)). However, since you only need the first matching rental, you don't need to call order, you could use.max_byinstead and get linear complexity.