Skip to content

Conversation

@pushpaagr
Copy link

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 read the seed data and we set the attributes to match seed file.
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 would be O(n) n being the number of customers.
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 find_by method which is using a hash to look up the data hence it would be O(1)
Describe a set of positive and negative test cases you implemented for a model. Our code tested for when all the valid parameters were placed it created a new movie, and another test was set to not create a movie with invalid parameters.
Describe a set of positive and negative test cases you implemented for a controller. For the Rental Controller--a Rental could be created with valid customer and movie rentals and a Rental could not be created with invalid customer or movie data.
How does your API respond when bad data is sent to it? We had a helper method called render_error that was called when bad data was sent and it would send json with error message and status.
Describe one of your custom model methods and why you chose to wrap that functionality into a method. Do you have any recommendations on how we could improve this project for the next cohort?
Link to Trello https://trello.com/invite/b/XbqKAXYt/1401ec6bdd844b89c272d5e7d0bc4898/pushpa-barbara
Link to ERD https://www.lucidchart.com/invitations/accept/7d00473a-1701-4f99-bbb6-17a8db112c9d
Summary This project provided us with a better understanding of how APIs work and also made our understanding of POSTMAN better. It also provided us with more practice of controller testing.

BarbaraWidjono and others added 30 commits November 5, 2018 11:19
@pushpaagr pushpaagr changed the title Barbara_Pushpa_Edges Barbara_Pushpa_VideoStoreAPI Nov 7, 2018
@Hamled
Copy link

Hamled commented Nov 16, 2018

Video Store

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene
Comprehension questions
Notes on the Big O questions:
* The O(n) answer for the index actions is probably fairly accurate, although I would like more explanation of your reason for that answer.
* The O(1) answer for find_by is understandable, although incorrect. The hash we pass as a parameter to find_by does indeed have linear access times, but what actually takes up "time" (in terms of work for the computer to do) is searching through the whole database table for matches. That is almost always longer than O(1), unfortunately. Through proper use of indexes we can get results that are something like O(n log n).
General
Business Logic in Models No.
All business logic is within the controller classes. I would suggest moving most of that code into the Rental model, or possibly Customer (e.g. with a Customer#rent_movie(movie) method).
All required endpoints return expected JSON data
Requests respond with appropriate HTTP Status Code
Errors are reported
Testing
Passes all Smoke Tests
Model Tests - all relations, validations, and custom functions test positive & negative cases
Controller Tests - URI parameters and data in the request body have positive & negative cases
Overall

decrease_movie_count(check_params[:customer_id])
rental = Rental.find_by(customer_id: check_params[:customer_id], movie_id: check_params[:movie_id])
if !rental
render json: { message: "Rental id: #{check_params[:id]} doesn't exist. Try again"}
Copy link

Choose a reason for hiding this comment

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

This should probably also have status: :not_found, if we're going to return an error message.

Alternatively, since this is a destroy action, we could just return a success response if the rental isn't found, since it's effectively deleted already. Technically that's what this line is doing since we're sending the default response code (OK), but it's probably a simpler interface for the "user" of the API if we keep the same success message even in this case.

temp = rental.id
temp_movie_id = rental.movie_id

if rental.destroy
Copy link

Choose a reason for hiding this comment

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

What happens if this call fails? We don't have an else case, so I'm not sure what we would end up rendering in that case.

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