Time - Corinna & Nataliya

[npogodina]

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 started by reading through the specifications laid out in the Read Me file before beginning to review the code itself. From there we read through each Ruby file individually before beginning to move back and forth between files to understand how they were linked. We used the TDD files to get a better understanding of what the program expected and how to best move forward making changes. Although we spent longer than some teams on Wave 0 and trying to understand the code, our process worked well for arming us with the knowledge we needed to move forward.
What inheritance relations exist between classes?	CSV_Record is the parent of Passenger, Driver and Trip because they all read from the original CSV files.
What composition relations exist between classes?	Trip Dispatcher is a driver class which runs the program and each trip has one driver and one passenger but drivers and passengers are connected to many trips.
Describe a decision you had to make when working on this project. What options were you considering? What helped you make your final decision?	When addressing the issue of pending rides we considered two options: creating a subclass that functioned as holding pen until rides were completed (therefore not effecting any of the standing tests) or amending classes and tests to allow for nil attributes. Ultimately, there wasn't enough uniqueness between the potential Pending class and the current Trip class to justify that addition and we decided to account for nil conditions in attributes of the existing Trip class.
Give an example of a template method that you implemented for this assignment	from_csv is a template method.
Give an example of a nominal test that you wrote for this assignment	We tested several nominal trips that contain all valid information and then tested to make sure that the result was not only an instance of the class we were expecting but also produced the correct data.
Give an example of an edge case test that you wrote for this assignment	Two Time focused edge case tests we wrote looked at start and end time and, more specifically, how to handle start and end time being the same (say, for example, if a trip was requested but then canceled) and how to handle incorrect chronological order.
What is a concept that you gained more clarity on as you worked on this assignment	Although we were only able to use it in a few places, reviewing our test code for areas we might be able to use the let variable to dry up our code helped us get a better understanding of when let is useful and when tests require individual attention.

[CheezItMan]

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	✔️
Practices inheritence. Driver inherits from CsvRecord, and implements from_csv	✔️
Employs problem-solving and implements Driver#average_rating and Driver#total_revenue	✔️, well done, I added a note on how to simplify this a bit.
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	⚠ You could do a better job making more granular commits. For example making a commit each time you complete a method or test case.

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)	✔️
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	✔️

Code Style Bonus Awards

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

Quality	Yes?
Perfect Indentation	✅
Descriptive/Readable	✅
Logical/Organized	✅

Summary

Well done, you hit all the learning goals here. Well done. Take a look at my inline comment, there are a few scenarios from Wave 2 that you didn't update once a driver/passenger could be on an in-progress trip. The existing tests work, but you don't test the methods with an in-progress trip. Otherwise excellent work!

[CheezItMan]

Since this is a test of the Passenger class, it makes more sense to test Passenger independently of Dispatcher, as you can have a Passenger without a dispatcher.

[CheezItMan]

You can also use some enumerables as well to simplify this a bit.

Suggested change

	trips.each do |trip|
	if trip.rating != nil
	finished_trips += 1
	rating_sum += trip.rating
	end
	complete_trips = trips.select { |trip| !trip.nil? }
	total = complete_trips.reduce(0) do |sum, trip|
	sum + trip.cost
	end
	return total / complete_trips.length.to_f

[CheezItMan]

Take a look at the above and see if you can identify how to use reduce to simplify this a bit.

[CheezItMan]

You should also include a test with a trip created without a driver or driver_id and similarly for passenger.

[CheezItMan]

You should also test this with a passenger who is on an incomplete trip.

[CheezItMan]

You should also test total_revenue to ensure it works if a driver has an in-progress trip.