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

Sai's ride share #23

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

Sai's ride share #23

wants to merge 43 commits into from

Conversation

ssamant
Copy link

@ssamant ssamant commented Mar 12, 2017

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? I needed to decide where to raise Argument Errors and then where to rescue them. I talked to a few people (incl. Kari and Bo) about where each of those things should happen in the code. Putting the rescue in the wrong spot and getting a crazy error helped me figure out where it really needed to go
Describe a concept that you gained more clarity on as you worked on this assignment. How methods fit together and figuring out which tasks belong to which class. The diagrams we used were super helpful
Describe a nominal test that you wrote for this assignment. Making sure the trips method in Driver returned an array of trips
Describe an edge case test that you wrote for this assignment. If a driver had no trips, making sure the return was nil
How do you feel you did in writing pseudocode first, then writing the tests and then the code? Pseudocode was very helpful -- helped me fix a couple pieces that weren't making sense in my diagram (e.g. how to get a list of drivers for a given rider). I still find it challenging to write the tests before the code, and I wasn't always successful in writing the appropriate tests, but I do think being forced to think through the big picture is ultimately helpful.

ssamant added 30 commits March 6, 2017 16:01
@kariabancroft
Copy link

kariabancroft commented Mar 14, 2017

Ride Share

What We're Looking For

Feature Feedback
Baseline
Used Git Regularly Yes
Answer comprehension questions Yes
Driver
Uses the all method in the find method Yes - nice job using the find enumerable method. I like the way you used the rescue here but I'd ask you which pieces you really needed to put inside the begin block. You end up duplicating a few of the variable assignments, so I think you could pull these out to DRY it up.
Has appropriate edge-case tests for each method in the class Yes - could anything go wrong with the rating method?
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 list to calculate the average rating Yes - nice & clear with the use of the local variables
Rider
Uses the all method in the find method Yes
Has appropriate edge-case tests for each method in the class Yes - good job
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 - nice job using the enumerables and preventing duplicates
Trip
Reads the CSV file in the all method Yes - similar comment about putting the code in the begin block. There is only one piece of this that needs to be in the begin since the rest will execute without exception
Has appropriate edge-case tests for each method in the class Yes
Created a method that uses a method from the Driver to retrieve the associated driver instance Yes
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 use of the find_all enumerable method
Overall You did a really nice job on this assignment. You clearly incorporated the major concepts we introduced and utilized them here. Nice job creating DRY tests, private methods and some custom exceptions and exception handling.

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