Skip to content
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

Stacks - Kelly Souza - RideShare Two #11

Open
wants to merge 33 commits into
base: master
Choose a base branch
from

Conversation

kellysouza
Copy link

Ride Share

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe a design decision you had to make when working on this project. What options were you considering? What helped you make your final decision? Which methods should be class vs instance. My decisions were based on who (which methods) were using that information and what information that method was being called on.
Describe a concept that you gained more clarity on as you worked on this assignment. Lots! calling methods from other class (e.g. calling method from class A within methods inside of class B. Initializing with a hash. Rescuing errors (I sort of created ways to rescue errors in my code just because I wanted to try it out).
Describe a nominal test that you wrote for this assignment. I tested to be sure, after initializing, the data for the first and last lines matched what was in the csv file.
Describe an edge case test that you wrote for this assignment. invalid ratings and invalid vins, I tested to be sure the initialize process worked as well as the other methods using that data, e.g. the average rating method.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I think I do really well with pseudocode. It is something that has always made sense to me and I have done as a habit prior to learning what it was called :)

# end
# end

describe "Creates drivers with valid vins" do

Choose a reason for hiding this comment

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

Is this a copy/paste issue?

@kariabancroft
Copy link

Ride Share

What We're Looking For

Feature Feedback
Baseline
Used Git Regularly Yes - though I'd encourage you to create more meaningful messages that describe the code you've written rather than just that you're getting tests to pass
Answer comprehension questions Yes
Driver
Uses the all method in the find method Yes - nice use of the find enumerable method. Note that you could DRY up the error handling logic on the creation of the Driver instance by putting only the piece that causes the error inside the begin block. Then for setting the vin to nil, you would only need to update the existing args variable instead of recreating the entire thing.
Has appropriate edge-case tests for each method in the class Mostly - I think the BadVin error handling should be within the overall initialize describe block rather than in a separate section. Would it be worthwhile to test a driver with no trips for both the trip method and the rating method?
Created a method that uses a method from the Trip object to retrieve the list of trips Yes - I think the driver_id variable is extraneous
Created a method that uses the internal trips list to calculate the average rating Yes - nice job utilizing some of the enumerable methods here
Rider
Uses the all method in the find method Yes
Has appropriate edge-case tests for each method in the class Mostly - same questions about the case where a rider didn't have any trips (or a trip didn't have a driver maybe)?
Created a method that uses a method from the Trip object to retrieve the list of trips Yes
Created a method that uses the internal trips method to retrieve the associated drivers Yes
Trip
Reads the CSV file in the all method Yes - same comment about which code to include in the begin block to DRY things up
Has appropriate edge-case tests for each method in the class Mostly - What about find a trip that doesn't exist? You have a test description that says "raises an error" but then the test assertion asserts nil - so it's be worth going through and making sure that the descriptions match the test content. LOTS of copy/paste issues in this file from Driver.
Created a method that uses a method from the Driver to retrieve the associated driver instance Yes - I'm curious why you'd want to search using the string rather than the integer. It seems like there is a mismatch of types somewhere along the line that should've been normalized.
Created a method that uses a method from the Rider to retrieve the associated rider instance Yes
Created a method to retrieve all trips by driver id Yes
Created a method to retrieve all trips by rider id Yes - nice job using the find_all enumerable method
Overall Naming convention note: the file names should always be singular (so driver.rb rather than drivers.rb and driver_spec vs drivers_specs).

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.

2 participants