Skip to content

Conversation

@lebaongoc
Copy link

@lebaongoc lebaongoc commented May 17, 2019

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

Question Answer
Explain how you came up with the design of your ERD, based on the seed data. We determined the compositions within our ERD based on the project requirement and the fields/columns based on the JSON file.
What would be the Big-O time complexity of your /customers & /movies endpoints? What does the time complexity depend on? Explain your reasoning. Both index endpoints have O(n) time complexity. The show method in movies controller has O(logn) time complexity while the create method has O(1) time complexity.
What is the Big-O time complexity of the POST /rentals/check-in endpoint? What does the time complexity depend on? Explain your reasoning. For the check-in endpoint, we have two helper methods to look up customer and movie by id so that takes O(2logn) and the other helper methods in the movie and customer model takes O(1) time so overall it is O(logn) time complexity.
Describe a set of positive and negative test cases you implemented for a model. In the movie model, in the custom method checkin, we tested with valid inputs, checkin would change the available inventory amount correctly (positive) and return false if given invalid input(negative).
Describe a set of positive and negative test cases you implemented for a controller. For the rentals controller, we tested the cases where a movie has available inventory - successfully checkout and where inventory is 0 - return error
How does your API respond when bad data is sent to it? it returns Json with the error message
Describe one of your custom model methods and why you chose to wrap that functionality into a method. We built a checkout_inventory method in the movie model which reduces the movie's available inventory by 1 if the movie has equal to or more than 1 movie for rent. We chose to wrap that functionality into a method to prevent the rentals controller on knowing too much about the movie object and make it easier to modify in the future if we decide to call that method in multiple places.
Do you have any recommendations on how we could improve this project for the next cohort? I enjoyed the project and don't have any recommendations at this point.
Link to Trello https://trello.com/b/8ETvaGMx
Link to ERD The ERD is an image attached in the Trello board
Summary

sjscotton and others added 30 commits May 14, 2019 12:02
…dded testing for movie controller - index method
@tildeee
Copy link

tildeee commented May 24, 2019

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene x
Comprehension questions x
General
Business Logic in Models x
All required endpoints return expected JSON data x
Requests respond with appropriate HTTP Status Code x
Errors are reported x
Testing
Passes all Smoke Tests x
Model Tests - all relations, validations, and custom functions test positive & negative cases x
Controller Tests - URI parameters and data in the request body have positive & negative cases x
Overall

Well done on this project, you two! The project submission is great-- not only is it well-organized, but I see places of the code being very elegant. Nice work! I'm particularly impressed with the code style in pulling the business logic out.

I've made a few comments of how to refactor and suggestions to get better.

Other than that, great work overall.

@@ -0,0 +1,37 @@
require "pry"
Copy link

Choose a reason for hiding this comment

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

Don't forget to get rid of this before submission! If anything, you likely wouldn't need to have this line, since pry is an included gem in the Gemfile

rental = Rental.new(movie_id: @movie.id, customer_id: @customer.id)
cur_date = Date.today
rental.checkout_date = cur_date
rental.due_date = cur_date + 7
Copy link

Choose a reason for hiding this comment

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

Maybe this above work (setting rental's checkout_date and due_date) can also be pulled into an instance method on Rental


has_many :rentals

def self.checkout_inventory(movie)
Copy link

Choose a reason for hiding this comment

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

Hm, maybe instead of making this a class method that takes in an instance of Movie as a parameter, it would work to make this an instance method. Then you would change the lines below to self.available_inventory

def self.checkout_inventory(movie)
if movie.available_inventory >= 1
movie.available_inventory -= 1
movie.save
Copy link

Choose a reason for hiding this comment

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

This is cool! Because of Ruby's implicit return, this method will return true if the movie saved successfully, or false if it didn't. Nice!

end
end

def self.checkin_inventory(movie, customer)
Copy link

Choose a reason for hiding this comment

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

As mentioned above, maybe this can be an instance method that only takes in customer

self.save
end

def self.checkin_movies_count(customer)
Copy link

Choose a reason for hiding this comment

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

Instead of making this a class method on Customer that takes in an instance, consider making this an instance method

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.

3 participants