Skip to content

Time - Denisse & Yaz#20

Open
denisseai wants to merge 9 commits into
Ada-C13:masterfrom
denisseai:master
Open

Time - Denisse & Yaz#20
denisseai wants to merge 9 commits into
Ada-C13:masterfrom
denisseai:master

Conversation

@denisseai

@denisseai denisseai commented Feb 29, 2020

Copy link
Copy Markdown

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 read through and diagramed the different class relationships - which ones were composition and which ones were inheritance. Having a visual representation was helpful as a reference when we were trying to understand the complexity.
What inheritance relations exist between classes? CsvRecord was the parent class to Trip, Passenger, and Driver. RideShare was the namespace for these classes.
What composition relations exist between classes? TripDispatcher has a one-to-many relationship with assenger, Trip, and Driver. Passenger is one-to-many to Trip, and Driver is one-to-many to Trip.
Describe a decision you had to make when working on this project. What options were you considering? What helped you make your final decision? In total_revenue we decided to use an array to calculate the revenue by mapping over trips, pulling the cost of each, and modifying based on revenue calculations. We did an array over a simple sum integer value. We made this decision out of instinct - which turned out to be right because we could thing about how to handle trips that where in progress without breaking this method.
Give an example of a template method that you implemented for this assignment from_csv in Driver.
Give an example of a nominal test that you wrote for this assignment For average rating we wrote a test for an example set of ratings to test the average for if it was a float and if it was the correct value.
Give an example of an edge case test that you wrote for this assignment If there were no trips, we returned 0 revenue for the driver.
What is a concept that you gained more clarity on as you worked on this assignment We gained more clarity on inheritance and composition. We also practice traceback and got quicker at finding where the error was located.

@beccaelenzil

Copy link
Copy Markdown

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 ✔️
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) See in line comment
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
Elegant/Clever
Descriptive/Readable

@beccaelenzil beccaelenzil left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great work overall. It is clear the learning goals around TDD, object composition, and inheritance were met. You did a particularly nice job breaking down complex functionality using helper methods. Keep up the hard work!

Comment thread lib/trip_dispatcher.rb
def request_trip(passenger_id)
driver = drivers.find {|driver| driver.status == :AVAILABLE }
if driver == nil
raise NoDriverError.new("No available drivers for this trip.")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice use of a custom exception!

Comment thread lib/trip_dispatcher.rb
driver_id: driver.id
)
trips << new_trip
driver.request_trip(new_trip)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice use of a helper method.

Comment thread lib/passenger.rb

# Calculate passenger's total time spent on trips
def total_time_spent
total_duration = trips.sum {|trip| trip.duration}

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 use of a enumerable.

Comment thread test/passenger_test.rb
it "returns 0 when trips is empty array" do
expect(@passenger.total_time_spent).must_equal 0
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.

These tests look good. You should test for one more edge case when the passenger has in progress trip.

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.

3 participants