Skip to content

space - Yieni & Cathy#16

Open
yieknee wants to merge 10 commits intoAda-C13:masterfrom
yieknee:master
Open

space - Yieni & Cathy#16
yieknee wants to merge 10 commits intoAda-C13:masterfrom
yieknee:master

Conversation

@yieknee
Copy link

@yieknee yieknee commented Feb 29, 2020

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 each class to see if we understood what was happening. What didn't work was starting at the abstract class. We also visually mapped out the interactions.
What inheritance relations exist between classes? Driver, Passenger, and Trip are subclasses of CSVRecord. TripDispatcher is its own class. We also created our own error that is a child of the StandardError Class.
What composition relations exist between classes? TripDispatcher is the driver code that connects the various classes.
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 wanted to decide how to handle the situation where no drivers are available, thus we created a NoDriverError that inherited from the StandardError class.
Give an example of a template method that you implemented for this assignment self.from_csv(record)
Give an example of a nominal test that you wrote for this assignment calculating the right float for the average rating given trips that had concluded.
Give an example of an edge case test that you wrote for this assignment return zero if there are no trips.
What is a concept that you gained more clarity on as you worked on this assignment object and inheritance.

@beccaelenzil
Copy link

OO Ride Share

Major Learning Goals/Code Review

Criteria yes/no, and optionally any details/lines of code to reference
Comprehension Questions Your answer about composition relationship gets at this. By composition we mean a has-a or has-many relationship. Examples in this project include "a TripDispatcher has-many Drivers, Passengers and Trips", "a Driver has-many Trips", "a Trip has-a Driver".
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) ✔️
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?
Elegant/Clever
Descriptive/Readable
Concise
Logical/Organized

Copy link

@beccaelenzil beccaelenzil left a comment

Choose a reason for hiding this comment

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

Great work on this project. The comprehensiveness of your tests is particularly notable. I've left a few in line comments for you to consider, but it is clear that the learning goals around TDD, object composition, and inheritance were met. Keep up the hard work!

end

def duration
if end_time != nil

Choose a reason for hiding this comment

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

Nice work handling in progress trips across the board (especially testing for this edge case!). I'm not sure if this would make sense with the rest of your implementation, but you might consider whether raising an exception instead of returning a string could work for in progress trips.

expect(@passenger.total_time_spent).must_equal 4500
end

it "Returns 0 if passenger has no trips" do

Choose a reason for hiding this comment

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

Great work testing for these 2 edge cases

end

def total_revenue
valid_costs = @trips.reject{|trip|trip.cost == nil}

Choose a reason for hiding this comment

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

Good use of an enumerable method

end
end

raise NoDriverError.new("Sorry! There are no available drivers. Please request a new trip.") if driver == nil

Choose a reason for hiding this comment

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

Good use of a custom exception

driver = nil
oldest_endtime = Time.now

available_drivers.each do |indiv_driver|

Choose a reason for hiding this comment

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

Good work implementing this logic. You may consider encapsulating this logic in a helper method.

end
end

describe "request_trip(passenger_id)" do

Choose a reason for hiding this comment

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

Nice work writing comprehensive tests for this method.

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