Skip to content

Time - Alex #23

Open
codesrobertson wants to merge 9 commits into
Ada-C13:masterfrom
codesrobertson:ar_branch
Open

Time - Alex #23
codesrobertson wants to merge 9 commits into
Ada-C13:masterfrom
codesrobertson:ar_branch

Conversation

@codesrobertson

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? Whiteboarding and creating a class diagram on draw.io was really helpful when I was first looking at all of the data. If I were to do something differently, I would have looked more-in-depth at the methods already in the file--the bottom-up approach along with the top-down approach.
What inheritance relations exist between classes? Passenger, Driver, and Trip all inherit from CsvRecord.
What composition relations exist between classes? Trip has a passenger and driver, and vice-versa.
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 I was working on this project, I had to decide whether or not to refactor code from the end of wave two. I was tempted to do so, but ultimately decided not to because of the timing, and our upcoming project, Hotel, which I believe will give me similar opportunities to practice and learn.
Give an example of a template method that you implemented for this assignment The self.from_csv method was an example of a template method I implemented in this assignment.
Give an example of a nominal test that you wrote for this assignment A nominal test that I wrote for this assignment was the "is an instance of passenger" test for the Passenger initialize method.
Give an example of an edge case test that you wrote for this assignment An edge case test that I wrote for this assignment was the "calculates total revenue when some trips cost less than $1.65" for the Driver total_revenue method.
What is a concept that you gained more clarity on as you worked on this assignment As I worked on this project, I gained greater clarity on how to develop tests. More than that, I realized how odd it felt to develop a test before writing the code for a method.

@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.

Good work persevering through this project. I have left a few inline comments for you to review. It is clear the learning goals around TDD, object composition, and inheritance were met. Keep up the hard work.

Comment thread test/passenger_test.rb
end
end

it "sums duration with incomplete trips" 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.

This is an important edge case to test for. Nice work identifying it. However, I do not see where you have an incomplete trip. An incomplete trip should have an end_time of nil. In addition, you should include this it block in the total_time_spent describe block above.

Comment thread test/passenger_test.rb
# You add tests for the net_expenditures method

it "sums up one trip's cost" do
@trip = RideShare::Trip.new(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The use of @ can be a little confusing in tests. It's only needed in a before block when you have a variable you want to use in multiple tests.

Comment thread lib/driver.rb
end

fees = (trips.count { |trip| (trip.cost && trip.cost >= 1.65) } * 1.65) +
(trips.reduce(0) { |total, trip| (trip.cost && trip.cost < 1.65) ? (total + trip.cost.to_f) : total })

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 use of an enumerable method. There is a lot going on in one line here. Considering breaking it up for readability.

Comment thread lib/trip_dispatcher.rb
@drivers.each do |driver|
if driver.status == :AVAILABLE
avail_driver = driver
break

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

instead of using a break here, consider whether there's an enumerable method or whether a while loop would work.

@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 You are missing tests for these edge casses.
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 ✔️Nice work written a helper method start_trip
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) You identified these cases as needing a test, but the test doesn't appear to actually test for incomplete trips. Correct me if I'm wrong!
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?
Elegant/Clever
Logical/Organized

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