Skip to content

Space - Faezeh & Lee#10

Open
theomoondev wants to merge 26 commits into
Ada-C13:masterfrom
theomoondev:master
Open

Space - Faezeh & Lee#10
theomoondev wants to merge 26 commits into
Ada-C13:masterfrom
theomoondev:master

Conversation

@theomoondev

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 decided to use the first day of the project to get familiar with the existing codebase and used a whiteboard/diagrams to check our understanding. It went well!
What inheritance relations exist between classes? Passenger, Trip, Driver are subclasses of CsvRecord, and inherit its methods.
What composition relations exist between classes? All of the classes belong to the RideShare module. TripDispatcher has Trips, Drivers, and Passengers. Trips have Drivers and Passengers, which are sibling 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 considered making additional helper methods to make the code a little more readable for humans, following a "people first" mentality. We made decisions about our shared working style to code only we're together, switch roles every half hour and take breaks, use Github to push and pull code onto each person's laptop. Clearly communicating with each other about our needs and our understanding of the requirements helped us make our final decision.
Give an example of a template method that you implemented for this assignment We used the template method CsvRecord#from_csv in the Driver subclass.
Give an example of a nominal test that you wrote for this assignment We tested whether the TripDispatcher#request_trip method created a trip properly.
Give an example of an edge case test that you wrote for this assignment We tested whether the TripDispatcher#find_available_driver method threw an argument error when there were no available drivers.
What is a concept that you gained more clarity on as you worked on this assignment Pair programming, how to explain code better, inheritance and composition (and how classes can interact with each other), TDD and how to write tests, how helpful pseudocode can be in preserving energy during the design process, and using pry to test methods and see how the existing code worked.

saraleecodes and others added 26 commits February 25, 2020 15:05
…es and total_time_spent methods - and method refactor
@CheezItMan

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 ✔️, I like your granular 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?
Perfect Indentation ✅, despite what github is showing 😕
Descriptive/Readable
Concise
Logical/Organized

Summary

Very well done, you hit the learning goals here. Nice work. I like especially how you tested the negative cases for incomplete trips etc. Nice work!

Comment thread lib/driver.rb
def initialize(id:, name:, vin:, status: :AVAILABLE, trips: nil)
super(id)

raise ArgumentError.new("Driver status is not valid") unless %i[AVAILABLE UNAVAILABLE].include?(status)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

weird, it's indented properly in VS code...

Comment thread lib/driver.rb
Comment on lines +31 to +36
ratings = []
@trips.each do |trip|
ratings << trip.rating unless trip.rating.nil?
end
return @trips.length == 0 ? 0 : (ratings.sum / ratings.length).to_f.round(1)
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.

Just note that you can simplify this type of thing with some enumerables. Also indentation is a little weird.

Suggested change
ratings = []
@trips.each do |trip|
ratings << trip.rating unless trip.rating.nil?
end
return @trips.length == 0 ? 0 : (ratings.sum / ratings.length).to_f.round(1)
end
completed_trips = trips.select { |trip| !trip.rating.nil? }
return 0 if completed_trips.length == 0
total_rating = completed_trips.reduce(0) { |sum, trip| sum + trip.rating }
return total_rating / completed_trips.length.to_f

Comment thread test/driver_test.rb
expect(RideShare::Driver.new(id: 100, name: "George", vin: "12345678901234567").status).must_equal :AVAILABLE
end

it "accepts all legal statuses" do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What about illegal statuses?

Comment thread test/driver_test.rb
expect(@driver.total_revenue).must_be_close_to 11.34, 0.01
end

it " does not include any in-progress trip" do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🥇

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.

2 participants