Skip to content

Space - Leah and Charlotte#17

Open
FreeMonkey19 wants to merge 28 commits into
Ada-C13:masterfrom
FreeMonkey19:master
Open

Space - Leah and Charlotte#17
FreeMonkey19 wants to merge 28 commits into
Ada-C13:masterfrom
FreeMonkey19:master

Conversation

@FreeMonkey19

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 took advantage of day 1 and did not write code. Instead we combed the files trying to understand the flow. This helped us prevent mistakes by understanding how everything was connected. This was an ongoing process for Charlotte.
What inheritance relations exist between classes? csv_record is the parent - passenger, trip and driver are the children
What composition relations exist between classes? passengers and drivers can have multiple trips but a passenger and trip has only one driver
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 had to decide how to handle nil, set up checks in the methods and add additional tests. we had to consider what we were going to return and decided in average_rating to return zero instead of nil. we thought it was more appropriate to return 0 rather than nil.
Give an example of a template method that you implemented for this assignment self.from_csv in the driver class
Give an example of a nominal test that you wrote for this assignment test to calculate net_expenditures in passenger_test
Give an example of an edge case test that you wrote for this assignment a lot of edge cases for dealing with nil: net_expenditures, total_time_spent, average_rating and total_revenue
What is a concept that you gained more clarity on as you worked on this assignment Charlotte: I have a better understand of classes overall - also instance objects. I also gained a better understanding of the importance of testing before coding. Leah: I gained a ton of clarity about how readers are METHODS and that working with classes is really just about calling methods over and over again. I also got some more clarity about why it's important to design a class structure with some flexibility so you can make changes without breaking everything!

leahwho and others added 28 commits February 24, 2020 18:50
…equirements; modify tests to account for those changes
@beccaelenzil

beccaelenzil commented Mar 12, 2020

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 You should use Time.parse when you read in the data so that you don't have to use it so many other places in your code. This helps DRY up your code.
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 I don't see tests of functionality for the situation where there is no available driver. Consider how to handle this edge case.

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
Descriptive/Readable

@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 on this assignment. I've left a few in line comments to review. Most of your methods are very thoroughly tests -- kudos! Request_trip and find_available_driver still code use a few edge case tests. However, it is clear that the learning goals around TDD, object composition, and inheritance were met. Keep up the hard work!

Comment thread lib/trip.rb
return self.new(
id: record[:id],
passenger_id: record[:passenger_id],
start_time: record[:start_time],

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As mentioned in the rubric feedback, it is strongly recommended to use Time.parse in this method to DRY up your code, and negate the need to use Time.parse elsewhere.

Comment thread test/passenger_test.rb
end


describe "net_expenditures with in-progress 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 a minor note. Typically a describe block is used for a single method, and separate it blocks are used for the different test cases for that method.

Comment thread lib/driver.rb
return 0
end

@trips.delete_if { |trip| trip.rating == nil }

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 use of an enumerable 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