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

Trang & Daniela - RideShare #5

Open
wants to merge 43 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
48ea89e
corrected trips_test.csv and outlined net_expenditures and total_time…
tfrego Aug 27, 2018
0dcb270
load_trips converts times into Time objects
tfrego Aug 27, 2018
43ead54
trip initialize raises ArgumentError if start time is later than end …
tfrego Aug 27, 2018
fb4aa26
calculate trip duration and completed tests
tfrego Aug 27, 2018
cc7d80b
calculates net expenditures for a user
mystioreo Aug 27, 2018
0cb375c
create total_time_spent method and associated tests
tfrego Aug 27, 2018
a603454
added outlines for new features
mystioreo Aug 27, 2018
845d78f
add ArgumentError for invalid ID in driver initialize
tfrego Aug 28, 2018
3f53a67
checks for valid VIN input, create empty driven_trips array
mystioreo Aug 28, 2018
eb1a4db
test for bad status
mystioreo Aug 28, 2018
cb369e4
add argument error for invalid driver status
tfrego Aug 28, 2018
71766c7
Merge branch 'master' of https://github.com/tfrego/oo-ride-share
tfrego Aug 28, 2018
5d70e53
create load_drivers and replace passenger with driver instance if dri…
tfrego Aug 28, 2018
1407316
find driver. updated tests to include third argument when creating a…
mystioreo Aug 28, 2018
5b3d0a6
stores a driver object for each trip
mystioreo Aug 28, 2018
ce6c971
create add_driven_trip method
tfrego Aug 29, 2018
83e9ef5
enabled tests in driver spec
mystioreo Aug 29, 2018
1da101c
Merge branch 'master' of https://github.com/tfrego/oo-ride-share
mystioreo Aug 29, 2018
aa88c2d
update driver_spec tests to driven_trip
tfrego Aug 29, 2018
30cbbcf
update trip_dispatcher_spec for driver
tfrego Aug 29, 2018
2676ef2
Merge branch 'master' of https://github.com/tfrego/oo-ride-share
mystioreo Aug 29, 2018
fa5d16f
wrote and tested average_rating method
mystioreo Aug 29, 2018
25d5961
wrote and tested Driver.average_rating
mystioreo Aug 29, 2018
43946e3
add total_revenue method for driver
tfrego Aug 29, 2018
d90eb63
wrote and tested net expenditures method
mystioreo Aug 29, 2018
d85e045
create request_trip method
tfrego Aug 29, 2018
12e4015
request trip adds the trip to both the passenger.trips and driver.dri…
mystioreo Aug 29, 2018
3dbdcda
raise ArgumentError if no drivers are available
tfrego Aug 29, 2018
46f4c11
updated old user and trip methods to accommodate in progress trips
tfrego Aug 30, 2018
da9ab8f
modified request_trip to choose the driver with no trips or the drive…
mystioreo Aug 31, 2018
6639641
added new trips csv to add data for wave 4
mystioreo Aug 31, 2018
d13cc32
fixed code that checks for the driver with the least recent trip
mystioreo Aug 31, 2018
216e930
format data to align
tfrego Aug 31, 2018
2aa53a8
Merge branch 'master' of https://github.com/tfrego/oo-ride-share
tfrego Aug 31, 2018
1350433
add more tests for Wave 4
tfrego Aug 31, 2018
7230306
refactored load users method
mystioreo Aug 31, 2018
9d724f7
refactored trip_dispatcher.rb
mystioreo Aug 31, 2018
cc05b13
refactor trip.rb
tfrego Aug 31, 2018
e2b7de4
refactor user.rb
tfrego Aug 31, 2018
1937225
refactored user_spec to use let
mystioreo Aug 31, 2018
26682f2
refactored trip_spec to use let
mystioreo Aug 31, 2018
e2f73fa
refactor driver_spec.rb
tfrego Aug 31, 2018
d75b2df
formatted trip_dispatch_spec
mystioreo Aug 31, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ net_expenditures|This method will **override** the cooresponding method in `User

**All the new methods above should have tests**

<!-- # Wave 3


Our program needs a way to make new trips and appropriately assign a driver and user.

Expand Down Expand Up @@ -245,7 +245,7 @@ Your code from waves 1 & 2 should _ignore_ any in-progress trips. That is to say

You should also **add explicit tests** for this new situation. For example, what happens if you attempt to calculate the total money spent for a `User` with an in-progress trip, or the average hourly revenue of a `Driver` with an in-progress trip? -->

<!-- # Wave 4
Wave 4

We want to evolve `TripDispatcher` so it assigns drivers in more intelligent ways. Every time we make a new trip, we want to pick drivers who haven't completed a trip in a long time, or who have never been assigned a trip.

Expand Down
15 changes: 12 additions & 3 deletions lib/trip.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,22 @@

module RideShare
class Trip
attr_reader :id, :passenger, :start_time, :end_time, :cost, :rating
attr_reader :id, :driver, :passenger, :start_time, :end_time, :cost, :rating

def initialize(input)
unless input[:end_time] == nil
raise ArgumentError.new "Start time must be before end time!" if input[:start_time] >= input[:end_time]
end
@id = input[:id]
@driver = input[:driver]
@passenger = input[:passenger]
@start_time = input[:start_time]
@end_time = input[:end_time]
@cost = input[:cost]
@rating = input[:rating]

if @rating > 5 || @rating < 1
raise ArgumentError.new("Invalid rating #{@rating}")
unless @rating == nil
raise ArgumentError.new("Invalid rating #{@rating}") if @rating > 5 || @rating < 1
end
end

Expand All @@ -22,5 +26,10 @@ def inspect
"ID=#{id.inspect} " +
"PassengerID=#{passenger&.id.inspect}>"
end

def trip_duration
return @end_time == nil ? nil : duration = @end_time - @start_time

Choose a reason for hiding this comment

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

Nice!

end

end
end
79 changes: 70 additions & 9 deletions lib/trip_dispatcher.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require 'csv'
require 'time'
require 'pry'

require_relative 'user'
require_relative 'trip'
Expand All @@ -9,46 +10,71 @@ class TripDispatcher
attr_reader :drivers, :passengers, :trips

def initialize(user_file = 'support/users.csv',
trip_file = 'support/trips.csv')
trip_file = 'support/trips.csv',
driver_file = 'support/drivers.csv')
@passengers = load_users(user_file)
@drivers = load_drivers(driver_file)
@trips = load_trips(trip_file)
end

def load_users(filename)
users = []

CSV.read(filename, headers: true).each do |line|
input_data = {}
input_data[:id] = line[0].to_i
input_data[:name] = line[1]
input_data[:phone] = line[2]

users << User.new(input_data)
users << User.new({:id => line[0].to_i, :name => line[1], :phone => line[2]})
end

return users
end

def load_drivers(filename)
drivers = []

CSV.read(filename, headers: true).each do |line|
found_passenger = find_passenger(line[0].to_i)

new_driver = Driver.new(id: line[0].to_i,
name: found_passenger.name,
phone: found_passenger.phone_number,
vin: line[1],
status: line[2].to_sym,
trips: found_passenger.trips)

drivers << new_driver
@passengers[@passengers.index(found_passenger)] = new_driver
end

return drivers
end

def find_driver(id)
check_id(id)
return @drivers.find { |driver| driver.id == id }
end

def load_trips(filename)
trips = []

trip_data = CSV.open(filename, 'r', headers: true,
header_converters: :symbol)

trip_data.each do |raw_trip|
passenger = find_passenger(raw_trip[:passenger_id].to_i)
driver = find_driver(raw_trip[:driver_id].to_i)

parsed_trip = {
id: raw_trip[:id].to_i,
driver: driver,
passenger: passenger,
start_time: raw_trip[:start_time],
end_time: raw_trip[:end_time],
start_time: Time.parse(raw_trip[:start_time]),
end_time: Time.parse(raw_trip[:end_time]),
cost: raw_trip[:cost].to_f,
rating: raw_trip[:rating].to_i
}

trip = Trip.new(parsed_trip)
passenger.add_trip(trip)
driver.add_driven_trip(trip)
trips << trip
end

Expand All @@ -67,6 +93,41 @@ def inspect
#{passengers.count} passengers>"
end

def request_trip(user_id)
available_drivers = @drivers.select {|driver| driver.status == :AVAILABLE and driver.id != user_id }

driver = available_drivers.find {|driver| driver.driven_trips.empty?}

Choose a reason for hiding this comment

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

Check your warnings. It doesn't make a huge amount of difference in THIS code, but here and on line 102, you have a "shadowed variable". driver on the left hand side of this assignment is being covered up by |driver| inside this find, and it creates a situation that is ambiguous to the compiler and ambiguous to a reader


# first find the latest trip end time for each driver, then find the driver associated with the least recent trip end time
driver ||= available_drivers.min_by {|driver|
latest_trip = driver.driven_trips.max_by {|trip| trip.end_time}
latest_trip.end_time}

raise ArgumentError, 'No drivers available' if driver == nil

passenger = find_passenger(user_id)

input = {
id: @trips.last.id + 1,
driver: driver,
passenger: passenger,
start_time: Time.now,
end_time: nil,
cost: nil,
rating: nil
}

new_trip = Trip.new(input)
@trips << new_trip

driver.make_unavailable
driver.add_driven_trip(new_trip)

passenger.add_trip(new_trip)

return new_trip
end

private

def check_id(id)
Expand Down
70 changes: 70 additions & 0 deletions lib/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,75 @@ def initialize(input)
def add_trip(trip)
@trips << trip
end

def net_expenditures
total = 0
@trips.each do |trip|
total += trip.cost unless trip.end_time == nil
end
return total
end

def total_time_spent
total_time = 0
@trips.each do |trip|
total_time += trip.trip_duration unless trip.end_time == nil
end
return total_time
end

end

class Driver < User

Choose a reason for hiding this comment

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

For future's reference, at least while you're still less experienced with Ruby, I'd avoid declaring 2 classes inside the same file. Especially in Ruby, this can hide problems with your code during the testing process.

attr_reader :vehicle_id, :driven_trips, :status, :trips

def initialize(id: 0, name: "no name", vin: 0, phone: 0, status: :UNAVAILABLE, trips: [])
raise ArgumentError.new "ID not valid" if id <= 0
raise ArgumentError.new "Invalid VIN" if vin.length != 17
raise ArgumentError.new "Invalid status" if status != :AVAILABLE && status != :UNAVAILABLE
@id = id
@name = name
@vehicle_id = vin
@phone = phone
@status = status
@driven_trips = []
@trips = trips
end

def average_rating
sum = 0
in_progress = 0

@driven_trips.each do |trip|
trip.end_time == nil ? in_progress += 1 : sum += trip.rating
end

valid_trips = @driven_trips.length - in_progress

return valid_trips == 0 ? 0 : sum.to_f/valid_trips
end

def add_driven_trip(trip)
raise ArgumentError, "Invalid trip object" unless trip.instance_of?(Trip)
@driven_trips << trip
end

def total_revenue
sum = 0

@driven_trips.each do |driven_trip|

Choose a reason for hiding this comment

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

Don't forget that in Ruby you have access to methods like Sum! I don't always think they're the best choice for every situation, but they can be an effective way to concisely communicate your thinking to readers!

sum += (driven_trip.cost - 1.65) * 0.8 unless driven_trip.end_time == nil
end

return sum.round(2)
end

def net_expenditures
return super - self.total_revenue
end

def make_unavailable
@status = :UNAVAILABLE
end
end
end
Loading