Skip to content

Queues- Danielle Birbal - Ride-Share-Two - #20

Open
birbalds wants to merge 9 commits into
Ada-C7:masterfrom
birbalds:master
Open

Queues- Danielle Birbal - Ride-Share-Two#20
birbalds wants to merge 9 commits into
Ada-C7:masterfrom
birbalds:master

Conversation

@birbalds

Copy link
Copy Markdown

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? Using the Trip class and its methods in the Driver and Rider classes to grab information was a design decision I made. I considered calling on the Driver class from the Rider class but to keep the program less tangled I stuck with only Trip.
Describe a concept that you gained more clarity on as you worked on this assignment. I further clarified the difference between CSV.open versus CSV.read
Describe a nominal test that you wrote for this assignment. My tests that verify the accuracy of the data upon initialization are nominal tests.
Describe an edge case test that you wrote for this assignment. One edge case I wrote tests for the first and last trips and whether data is accurately manipulated for them.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I did poorly because my pseudocode quickly turned into actual code as I wrote it.

@droberts-sea

Copy link
Copy Markdown

Ride Share

What We're Looking For

Feature Feedback
Baseline
Used Git Regularly You've made a good start here, but I'd like even more details. Rather than "Added Driver and Trip functionality", something like "Implemented Driver#average_raiting" would be good. That might mean you have to break your work down into smaller commits.
Answer comprehension questions Regarding the last question: on class projects like these, it often feels like it's not worth spending the time pseudocoding or diagramming when you can see the solution and just go right to it. However, these skills will be invaluable when you get out into the real world and are faced with really difficult problems. It's definitely worth your while to slow down and practice them now, when the problem scope is relatively small.
Driver see inline comments
Uses the all method in the find method yes
Has appropriate edge-case tests for each method in the class missing one - see inline comment
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
Rider
Uses the all method in the find method yes
Has appropriate edge-case tests for each method in the class mostly - see inline
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
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
Overall

Great job! Code is clear and readable, and tests cover almost everything I was looking for. Keep up the good work!

Comment thread lib/driver.rb

DRIVER_INFO.each do |line|
next unless line[0].to_i == id
@driver_id = id

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I like the idea here - being able to initialize a driver with just the ID and have the program fill in the details is very convenient. However, I'm a little worried about the efficiency of looping through the whole DRIVER_INFO array here. In particular, if Driver.all creates a new Driver for every ID in DRIVER_INFO, then Driver.all ends up being O(n^2).

It might be you could chose a different data structure to get around this. I'll leave that up to you.

Comment thread lib/driver.rb
end

def avg_rating
@driver_trips = trips

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm curious why you chose to use an instance variable here. Seems like the trips don't need to persist long term, only for the duration of the method, which to me indicates that a local variable might be a better choice.

Comment thread specs/driver_spec.rb
end
it 'Calculates the average rating correctly' do
new_driver.avg_rating.must_equal 3.4
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What happens if the driver hasn't made any trips yet?

Comment thread specs/driver_spec.rb
describe RideShare::Driver do
let(:new_driver) { RideShare::Driver.new(18) }

describe 'initialize' do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good organization - the nested describes make this code much easier to read.

Comment thread specs/rider_spec.rb
new_rider.previous_drivers[0].driver_id.must_equal 69
new_rider.previous_drivers[0].name.must_equal 'Ernesto Torp'
new_rider.previous_drivers[0].vin.must_equal 'RF4BPA803R4AACTR1'
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As with Driver.average_rating, what happens if the rider hasn't taken any trips?

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