Skip to content

Comments

Dora and Janice#24

Open
jaitch wants to merge 47 commits intoAda-C12:masterfrom
jaitch:master
Open

Dora and Janice#24
jaitch wants to merge 47 commits intoAda-C12:masterfrom
jaitch:master

Conversation

@jaitch
Copy link

@jaitch jaitch commented Nov 8, 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 considered what field we needed, not only for the seed data but going forward with the project (check-in and check-out dates, for example). We decided to use a 'has_many through' relationship because we knew each customer could have many rentals and each movie could belong to many rentals depending on its available inventory. Because rentals had their own attributes (check-out, check-in and due dates), it could not be just a join table.
What would be the Big-O time complexity of your /customers & /movies endpoints? What does the time complexity depend on? Explain your reasoning. The time complexity depends on the number of items that need to be returned or searched through, one iteration for each customers or movies. Here, Big O would be O(n). Worst case scenario is that the whole collection would have to be searched through.
What is the Big-O time complexity of the POST /rentals/check-in endpoint? What does the time complexity depend on? Explain your reasoning. The time complexity depends on the time required to find the customer (O(n)), the time required to find the movie (O(n)), and the time required to locate the rental (O(1)). O(2n+1) would become O(n).
Describe a set of positive and negative test cases you implemented for a model. For our validation tests within the movie model, we tested that a movie could be created with a valid title (and other required validations) and could not without a valid title (and other required validations).
Describe a set of positive and negative test cases you implemented for a controller. For the show action in the movie controller, we tested that it responds with a movie's hash given a valid ID; with invalid data, it returns an error message (not found).
How does your API respond when bad data is sent to it? It renders a JSON with the appropriate message-- 'not found ' or 'bad request'.
Describe one of your custom model methods and why you chose to wrap that functionality into a method. In our movie model, we have a custom method that populates the available inventory field with the existing value of inventory upon creation of a movie instance. We made that a method so we could use the after_initialize filter so it would only get called when a new movie was successfully made.
Do you have any recommendations on how we could improve this project for the next cohort? Coordinate the requirements and tests so that optionals aren't included in the required testing. Smaller bite-size optionals that would be easier to tack on at the end of the week.
Link to Trello https://trello.com/b/6G3m8NAq/video-store-api
Link to ERD https://www.lucidchart.com/documents/edit/17b23890-1d43-4224-bbe4-dbd2ec713334/0_0
Summary Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. << LOL

janicewhuang and others added 30 commits November 5, 2019 13:14
…in customer controller, add dependent nullify to customer and movie models
@CheezItMan
Copy link

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good number of commits and mostly good commit messages
Comprehension questions Ipsem dolor... lol
General
Business Logic in Models None
All required endpoints return expected JSON data Check
Requests respond with appropriate HTTP Status Code Check
Errors are reported Check
Testing
Passes all Smoke Tests Check
Model Tests - all relations, validations, and custom functions test positive & negative cases Check
Controller Tests - URI parameters and data in the request body have positive & negative cases Check
Overall Nice work, you hit the learning goals here. The only major comment I have is that business logic isn't in the models, but rather resting in the rentals controller. Something to think about. Otherwise, excellent job.

class Movie < ApplicationRecord
has_many :rentals, dependent: :nullify
has_many :customers, through: :rentals, dependent: :nullify
after_initialize :set_available_inventory, only: [:save], if: :new_record?

Choose a reason for hiding this comment

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

clever

@@ -0,0 +1,4 @@
class Rental < ApplicationRecord

Choose a reason for hiding this comment

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

I'd suggest moving some business logic into the rental class.

@@ -0,0 +1,68 @@
class RentalsController < ApplicationController
def checkout

Choose a reason for hiding this comment

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

I suggest moving some of the business logic into the models, these methods are quite long.

must_respond_with :ok
end

it "increases movie's inventory by one if successfully checked back in" do

Choose a reason for hiding this comment

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

What if there is not enough inventory left to be checked out?

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.

4 participants