Skip to content

Time- Mair and Yesenia#18

Open
mheshmati-tech wants to merge 8 commits intoAda-C13:masterfrom
mheshmati-tech:master
Open

Time- Mair and Yesenia#18
mheshmati-tech wants to merge 8 commits intoAda-C13:masterfrom
mheshmati-tech:master

Conversation

@mheshmati-tech
Copy link

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? It took us both a long time to understand the datasets and existing classes/tests. Because there was so much interaction going on between the files, it was hard for us to grasp what was going on. We're hoping that overtime we develop the skills to understanding code we haven't written.
What inheritance relations exist between classes? Driver and Passenger are subclasses of CSV Record.
What composition relations exist between classes? Trip Dispatcher has a one to many relationship with Driver and Passenger. Driver has one to many relationship with Trips. Passenger has one to many relationship with Trips. Within a trip, Driver and Passenger have a one to one relationship.
Describe a decision you had to make when working on this project. What options were you considering? What helped you make your final decision? For Trip Dispatcher tests specifically, we had to figure out how to access the test data. We decided to replicate the way it had been done by the original writers of the tests, since it was difficult for us to comprehend.
Give an example of a template method that you implemented for this assignment We created template method with net_expenditure for the Passenger class.
Give an example of a nominal test that you wrote for this assignment We created nominal tests for the request_trip method, where we tested to see if it was instantiated correctly.
Give an example of an edge case test that you wrote for this assignment For the request_trip method, we created a test that would raise an argument error when there were no drivers with an available status.
What is a concept that you gained more clarity on as you worked on this assignment Mair gained clarity on testing and how to use before/let correctly. Yesenia gained clarity on inheritance and references/values.

@beccaelenzil
Copy link

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 ✔️You could use a few more smaller commits. In particular, Wave 1 probably could have had a few more commits along the way.

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) Both your tests and your method implementation need to consider incomplete 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
Elegant/Clever

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 assignment. I've left a few comments for you to review. It is clear that the learning goals around TDD, object composition, and inheritance were met. Keep up the hard work!

trip_ratings += trip.rating
end

if trip_ratings > 0

Choose a reason for hiding this comment

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

Consider whether this conditional is necessary.

found_passenger = @passengers.find do |passenger|
passenger.id == passenger_id
end
p_id = found_passenger.id # should be the same as the method's argument

Choose a reason for hiding this comment

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

You're right! It should be the same as the argument. In that case, do you need this variable assignment?

raise ArgumentError "No driver is available, sorry!"
end

d_id = available_driver.id # get their id

Choose a reason for hiding this comment

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

Consider naming this variable driver_id to increase readability.

# update the Driver and Passenger objects for this trip
found_passenger.add_trip(in_progress_trip)
available_driver.add_trip(in_progress_trip)
available_driver.switch_status

Choose a reason for hiding this comment

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

Great use of a helper method.

end
end

describe "request_trip method" do

Choose a reason for hiding this comment

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

Great test coverage 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