Skip to content

Haben, Hannah, Angela - TIME #22

Open
HabenFoto wants to merge 7 commits into
Ada-C13:masterfrom
HabenFoto:master
Open

Haben, Hannah, Angela - TIME #22
HabenFoto wants to merge 7 commits into
Ada-C13:masterfrom
HabenFoto:master

Conversation

@HabenFoto

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? It was a lot more difficult than anticipated. At first, we read over the project document and it felt like we had a pretty good grasp on how the classes interact with each other. But as we went on to write code and tests, our understanding of the class interactions became more and more confused. What went well - talking through our understandings and also our confusions with a whiteboard to support our guesses helped a lot.
What inheritance relations exist between classes? Driver, Trip, and Passenger class all inherit from Csvrecord to drive data structuring.
What composition relations exist between classes? A Trip class can store Driver and Passenger objects in 1-to-1 relationships. Drivers and Passengers each have an array of Trips in 1-to-many (one Driver has many Trips, one Passenger has many trips).
Describe a decision you had to make when working on this project. What options were you considering? What helped you make your final decision? At a lot of points, we deliberated between whether to use Enumerables or .each loops and ultimately made our decisions based on what kind of data we were trying to find.
Give an example of a template method that you implemented for this assignment Csv_record's .from_csv method is a template method that we implemented in its subclasses.
Give an example of a nominal test that you wrote for this assignment We wrote a nominal test to check if a new Trip object was being created correctly.
Give an example of an edge case test that you wrote for this assignment We wrote an edge case test to check for situations where end time occurs before start time.
What is a concept that you gained more clarity on as you worked on this assignment We learned a lot about classes and inheritance and how complicated their relationship is. Importance of grouping similar ideas together, both in writing classes and tests.

@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 ⚠ There's no test for a passenger with 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 should make more granular commits, I suggest after completing each method or test.

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 ✔️
There is reasonable test coverage for TripDispatcher#request_trip, and all tests pass ✔️, except for one skipped test, see my inline notes on that.

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?
Descriptive/Readable
Logical/Organized

Summary

You have some minor indentation issues here, like in lines 23-28 in trip_test.rb. Nice work, you hit the main learning goals here. Well done in waves 1-3. Take a look at my inline comments and let me know if you have any questions.

Comment thread test/trip_test.rb
end

it "stores an instance of driver" do
skip # Unskip after wave 2

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 noting this is skipped and so the Trip instance doesn't have a reference to the driver for the trip.

Comment thread test/trip_test.rb
driver_id: 2
}
@trip = RideShare::Trip.new(@trip_data)
@driver = RideShare::Driver.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.

You should connect this driver to @trip to use in the skipped test.

Comment thread lib/driver.rb
Comment on lines +30 to +36
total_rating = 0
number_of_finished_trips = 0
@trips.each do |trip|
if trip.end_time != nil
total_rating += trip.rating
number_of_finished_trips += 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 noting you can simplify this a bit with enumerables

Suggested change
total_rating = 0
number_of_finished_trips = 0
@trips.each do |trip|
if trip.end_time != nil
total_rating += trip.rating
number_of_finished_trips += 1
end
return 0 if trips.length == 0
completed_trips = trips.select { |trip| !trip.rating.nil? }
total_ratings = completed_trips.reduce(0) do |sum, trip |
sum + trip.rating
end
return total_ratings / completed_trips.length.to_f

Comment thread lib/driver.rb
end
end

def total_revenue

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Well done, This works well.

I suggest you look up and see how you could adapt the code I wrote above, just for practice using enumerables.

Comment thread lib/trip.rb
Comment on lines +92 to +93
start_time: Time.parse(record[:start_time]).utc, # addedd Time.parse and utc to change the time from csv
end_time: Time.parse(record[:end_time]).utc, #style which is a string to give us time object

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