-
Notifications
You must be signed in to change notification settings - Fork 28
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
Aurea & Anibel - Edges #1
base: master
Are you sure you want to change the base?
Conversation
Ride ShareWhat We're Looking For
Great work on this project you too! Your code is really clear and readable-- in the bits with complex logic, the code ends up being very clear because of the appropriate use of The ternaries really paid off in this code base-- everything looks great You made all of your instance variables in every class read-only, and that makes me really happy! Adding a few comments for suggestions, but otherwise good job |
def total_revenue | ||
valid_trips = @driven_trips.find_all { |trip| trip.cost } | ||
|
||
return (valid_trips.sum { |trip| trip.cost - 1.65} * 0.80).round(2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd probably pull these "magic number" integers out into constants
@passengers = load_users(user_file) | ||
@drivers = load_drivers(driver_file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done loading drivers before trips!
|
||
it 'selects a driver with an available status' do | ||
driver = @dispatcher.find_driver(8) | ||
num_of_trips = driver.driven_trips.length |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copy/pasta! num_of_trips
doesn't get used in this test
|
||
driver_5.add_driven_trip(trip3) | ||
|
||
expect( @dispatcher.assign_driver.id ).must_equal 8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job with these tests and making the test data yours so you can use concrete answers
OO Ride Share
Congratulations! You're submitting your assignment!
Comprehension Questions
duration
in theTrip
class, we decided that for any in-progress trips, it would evaluate tonil
as opposed to0
. We usednil
as an indication of in-progress trips in our other methodtotal_time_spent
.User
has a one to many relationship toTrip
, and eachTrip
has a one to one relationship with aDriver
, and a one to one or one to many relationship withUser
as a passenger.User
andDriver
Driver
inheritsUser
attributes and behaviors. It is a direct child of theUser
class.trip_dispatcher_spec.rb
, in theassign_driver
method tests starting on line 177, we wrote tests that checked to make sure that it returns the driver with the oldest most recent trip.trip_dispatcher_spec.rb
, the test starting in line 136, we check to see thatrequest_trip
returnsnil
if no available drivers.load
methods for the three classes, it became apparent to think about the order in which we loaded csv files since the classes interacted with each other.