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

Ann's Ride Share Two project #18

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

Conversation

gofarann
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? I had to decide whether user had the ability to instantiate individual rider/driver/trips and add to the CSV file. I made the decisions of no, because I saw the project as a code to read into the CSV database.
Describe a concept that you gained more clarity on as you worked on this assignment. That the .all method creates instances of the class object, and that the class objects are what the methods are being called on. Additionally, I also gained more understanding of the CSV manipulation techniques through creating a CSV from scratch
Describe a nominal test that you wrote for this assignment. One nominal test was to find rating of a particular driver. I picked a driver with trips and ran the method to make sure it returned a float.
Describe an edge case test that you wrote for this assignment. One edge test was to find the rating of a driver with no trips, and thus no ratings. This prompted the Argument error that was in the code.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I started off writing psedocode for the driver class... I felt ok about it. The subsequent classes, I did not write pseudocode for because they were iterations of similar class methods and syntax as the driver class. In hindsight, I would've liked to write those out as well.

it "raises an error when an invalid id is given" do
proc {
Rideshare::Driver.find(1000)
}.must_raise ArgumentError

Choose a reason for hiding this comment

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

This is probably better to return nil as opposed to an Error as it's not really an error, but rather nothing was found.


def self.all_drivers #method to return the class variable drivers
return @drivers
end

Choose a reason for hiding this comment

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

This seems to be accessing an instance variable, not a class variable....

Rideshare::Trips.all_trips.each do |trip|
if trip.driver_id == @id #both are integers now, still not working
driver_rating << trip.rating #rating must be integer
end

Choose a reason for hiding this comment

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

You could create a method in Trips to return a list of Trips for a specific Driver ID.

@CheezItMan
Copy link

Ride Share

What We're Looking For

Feature Feedback
Baseline
Used Git Regularly You still seem to write in big commits. It makes it a little harder to see the steps in your progress.
Answer comprehension questions What do you mean the user couldn't create a Driver or Trip or Rider instance? They have `initialize methods. It also sounds like you had trouble getting into pseudo-code.
Driver
Uses the all method in the find method Check, but it was specific to the absolute path
Has appropriate edge-case tests for each method in the class You didn't check finding the 1st and last Driver.
Created a method that uses a method from the Trip object to retrieve the list of trips Check
Created a method that uses the internal trips list to calculate the average rating Check, but with both this and the line above you could create a method in Trips to find all the trips for a specific driver as opposed to the other way around.
Rider
Uses the all method in the find method Check
Has appropriate edge-case tests for each method in the class You should probably also test for the correct sum in the all_spend method.
Created a method that uses a method from the Trip object to retrieve the list of trips Check
Created a method that uses the internal trips method to retrieve the associated drivers Check, but you could have created a method in Trips to find the trips for a specific driver as opposed to the other way around.
Trip
Reads the CSV file in the all method Check
Has appropriate edge-case tests for each method in the class You also want to check the correct number of trips returned for Trips#all_by_driver as well as checks for when the driver ID doesn't exist
Created a method that uses a method from the Driver to retrieve the associated driver instance Check
Created a method that uses a method from the Rider to retrieve the associated rider instance Check
Created a method to retrieve all trips by driver id Check
Created a method to retrieve all trips by rider id Check

Summary

Overall nicely done, you've come a long way and did a good job creating methods. Somethings to practice are using enumerable methods and making sure you test all the edge cases in your classes.

Rideshare::Driver.all
Rideshare::Trips.all
Rideshare::Rider.all
end

Choose a reason for hiding this comment

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

Do you need to do all three 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