Water: Leah & Sophia

[SoCodeDo]

Assignment Submission: OO Ride Share

Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.

Reflection

Question	Answer
How did getting up to speed on the existing codebase go? If it went well, what worked? If it didn't go well, what will you do differently next time?	We worked through each method and made sure we understood what was happeing in our code. Instead of jumping into wave 1, we took the first day to go step by step through wave 0 and make sure we completely understood the code base we had.
What inheritance relations exist between classes?	The other classes inherit from CSV record.
What composition relations exist between classes?	Drivers and Passengers can have many trips, but a trip can only have one driver and one passenger.
Describe a decision you had to make when working on this project. What options were you considering? What helped you make your final decision?	We went with the simplest solution. We leaned towards more complicated solutions and then through working with a TA we were able to see how our code could be simplified and cleaner.
Give an example of a template method that you implemented for this assignment
Give an example of a nominal test that you wrote for this assignment	it "returns net expenditures for each passenger's rides" do
expect(@passenger.net_expenditures).must_equal 4
end
Give an example of an edge case test that you wrote for this assignment	it "returns 0 if there are no trips" do
@driver = RideShare::Driver.new(

  id: 32,
  name: "Colorful Blob",
  vin: "DKJSEKVKDKFJEKRKT"

)
expect(@driver.total_revenue).must_equal 0
end
What is a concept that you gained more clarity on as you worked on this assignment | TDD, we both gained a better understanding of how to write and read tests.

[kaidamasaki]

Great job! It looks like you need to practice your edge case tests a little more but other than that everything looked good!

OO Ride Share

Major Learning Goals/Code Review

Criteria	yes/no, and optionally any details/lines of code to reference
The code demonstrates individual learning about Time and the responsibility of Trip.from_csv, and uses Time.parse in Trip.from_csv	✔️
The code demonstrates breaking out complex logic in helper methods, such as making a helper method in Trip to calculate duration	✔️
There are tests for the nominal cases for the Passenger#net_expenditures and Passenger#total_time_spent	✔️
There is at least one edge case test for either Passenger#net_expenditures or Passenger#total_time_spent testing if the passenger has no trips	No edge case test.
Practices inheritance. Driver inherits from CsvRecord, and implements from_csv	✔️
Employs problem-solving and implements Driver#average_rating and Driver#total_revenue	✔️
Implements the TripDispatcher#request_trip, which creates an instance of Trip with a driver and passenger, adds the new trip to @trips, and changes the status of the driver	✔️
Practices composition. In TripDispatcher#request_trip, the driver gets connected to the new trip, the passenger gets connected to the new trip	✔️
Practices git with at least 10 small commits and meaningful commit messages	✔️

Testing Requirements

Testing Requirement	yes/no
There is reasonable test coverage for wave 1, and all wave 1 tests pass	✔️
There is reasonable test coverage for wave 2, and all wave 2 tests pass	✔️
Wave 3: Tests in wave 1 and wave 2 explicitly test that only completed trips should be calculated (and ignore in-progress trips)	Only tests nominal case.
There is reasonable test coverage for TripDispatcher#request_trip, and all tests pass	✔️

Overall Feedback

Overall Feedback	Criteria	yes/no
Green (Meets/Exceeds Standards)	8+ in Code Review && 3+ in Functional Requirements	✔️
Yellow (Approaches Standards)	6+ in Code Review && 2+ in Functional Requirements
Red (Not at Standard)	0-5 in Code Review or 0,1 in Functional Reqs, or assignment is breaking/doesn’t run with less than 5 minutes of debugging

Code Style Bonus Awards

Was the code particularly impressive in code style for any of these reasons (or more...?)

Quality	Yes?
Perfect Indentation	✅
Elegant/Clever	✅
Descriptive/Readable	✅
Concise	✅
Logical/Organized	✅

[kaidamasaki]

It's helpful for debugging to include the bad argument in your variable:

Suggested change

	raise ArgumentError.new("Invalid VIN") if (vin.length) != 17
	raise ArgumentError.new("Invalid VIN: #{vin}") if (vin.length) != 17

[kaidamasaki]

You might want to verify that the driver is available before accepting a trip.

[kaidamasaki]

You should move this check up. If @trips is nil you will get an error when you call @trips.reject:

Suggested change

	def average_rating
	ratings_array = @trips.reject { |trip| trip.rating == nil }.map(&:rating)
	return 0 if (@trips) == [] || @trips == nil
	return (ratings_array.sum/ratings_array.length).to_f
	end
	def average_rating
	return 0 if (@trips) == [] || @trips == nil
	ratings_array = @trips.reject { |trip| trip.rating == nil }.map(&:rating)
	return (ratings_array.sum/ratings_array.length).to_f
	end

[kaidamasaki]

Great use of enumerable methods!

[kaidamasaki]

Nice job including the bad argument here!

[kaidamasaki]

You can directly return @trips.sum(&:cost):

Suggested change

	total_expenditure = @trips.sum(&:cost)
	return total_expenditure
	return @trips.sum(&:cost)

(This saves you from having to come up with a variable name.)

[kaidamasaki]

You got a warning for this. To fix it directly use keyword arguments instead of passing in a hash:

Suggested change

	trip_data = {
	id: @trips.length + 1,
	passenger: passenger,
	start_time: start_time,
	end_time: end_time,
	cost: nil,
	rating: nil,
	driver_id: driver.id,
	driver: driver
	}
	new_trip_instance = RideShare::Trip.new(trip_data)
	new_trip_instance = RideShare::Trip.new(
	id: @trips.length + 1,
	passenger: passenger,
	start_time: start_time,
	end_time: end_time,
	cost: nil,
	rating: nil,
	driver_id: driver.id,
	driver: driver
	)