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 - ride share two - Jackie #40

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

Conversation

jackiewatanabe
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? While working on this project, I realized I was struggling to write edge test cases because the code was so dependent on this one particular file. I thought about writing it in a way so that the code didn't depend on this particular csv file but was not sure where I'd be able to store the data and also stick to the requirements.
Describe a concept that you gained more clarity on as you worked on this assignment. the distinction between class and instance methods and when to use them. Class methods are not tied to any one instance of a class. Instance methods can only be used on particular instances of the class
Describe a nominal test that you wrote for this assignment. in Driver.find, I test to make sure the correct driver is returned if a valid id number is entered.
Describe an edge case test that you wrote for this assignment. in Driver.find, I test to make sure an argument is raised when an id that does not exist is entered. As I said above, I really struggled writing tests for this project, and I found it difficult to test edge cases when most of the methods were tied to a specific csv file...
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I could have done a lot better with writing pseudocode first. I did draw diagrams, which helped give me a visual of what my code should end up looking like.

…s. also added all class method for Driver that reads in a csv file and returns a list of all drivers.
…ch all trips based on one rider id. Also added tests to .find class methods for rider and driver classes to make sure argument errors are raised if search returns no matches
…in Trip class to search for all trip instances based on one driver id.
@kariabancroft
Copy link

kariabancroft commented Mar 13, 2017

Ride Share

What We're Looking For

Feature Feedback
Baseline
Used Git Regularly Yes - good job
Answer comprehension questions Yes - glad you gained more clarity on class vs instance methods
Driver
Uses the all method in the find method Yes. Not finding the driver is not really an argument error case (per our discussion on errors) so I'd argue that you'd want to give the user some alternate information in this case
Has appropriate edge-case tests for each method in the class Yes. I might add: What would average return if there were no trips associated? Style tip: When you have a bunch of closing end tags in a row, they should not be separated out by extraneous whitespace.
Created a method that uses a method from the Trip object to retrieve the list of trips Using the all method to search through the list of all and retrieve the associated trips. I'd argue that this would be better to have as a method in the Trip class which would just give you back the trips, instead of searching through all the trips in the Driver class.
Created a method that uses the internal trips list to calculate the average rating Yes. You can clean up your return by excluding the rating = piece since you won't be utilizing this variable anywhere after the return.
Rider
Uses the all method in the find method Yes. What enumerable method could you use in find rather than what you're doing with the each?
Has appropriate edge-case tests for each method in the class Yes - these look good
Created a method that uses a method from the Trip object to retrieve the list of trips Yes. It looks like you're doing what I suggested above in the Driver class to use the other class to take care of that logic for you.
Created a method that uses the internal trips method to retrieve the associated drivers Yes. This will work but I think you're potentially doing an extra step. First, you can get the trips, then on each trip you should be able to retrieve the Driver (trip.driver) rather than going through each ID and calling the Driver.find method.
Trip
Reads the CSV file in the all method Yes
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 No - this should exist and be using the Driver.find method using the @driver ID to pull in the Driver object. Now I see why my above suggestion about the trips wouldn't work.
Created a method that uses a method from the Rider to retrieve the associated rider instance No - this should exist and be using the Rider.find method using the @rider ID to pull in the Rider object.
Created a method to retrieve all trips by driver id Yes - but you're not using the method anywhere (only in the tests)
Created a method to retrieve all trips by rider id Yes - each of these methods have an opportunity to use an enumerable method which would simplify the code a bit
Overall Nice job. You wrote a lot of good tests and code throughout this assignment. You have a few more opportunities to use enumerable methods, but you did a good job with what you created here.

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