-
Notifications
You must be signed in to change notification settings - Fork 29
Ross and Emily - Time #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
… that the method is working properly.
beccaelenzil
left a comment
There was a problem hiding this 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! It is clear that the learning goals around TDD, inheritance, and object composition were met. In particular, nice job implementing the optional features and writing comprehensive tests for this. One thing to note is you could use more tests for the edge case on the passenger methods. Keep up the hard work.
| end | ||
|
|
||
| # You add tests for the net_expenditures method | ||
| it "will return the total cost for all trips made by the passenger" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests are good, but a few more are needed. What happens if a passenger has zero trips? What happens if the passenger has trips that are in progress?
| end | ||
| end | ||
|
|
||
| describe "Request Trip method tests" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work on these tests!
| return 0 | ||
| end | ||
|
|
||
| @trips.each do |trip| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using a .map
| end | ||
|
|
||
| available_drivers.each do |driver| | ||
| if driver.trips.length == 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an enumerable you could use here?
OO Ride ShareMajor Learning Goals/Code Review
Testing Requirements
Overall Feedback
Additional FeedbackYour answer to the comprehension question about composition gets at this, but I want to call out this vocabulary explicitly. By composition we mean a has-a or has-many relationship. Examples in this project include "a TripDispatcher has-many Drivers, Passengers and Trips", "a Driver has-many Trips", "a Trip has-a Driver". Code Style Bonus AwardsWas the code particularly impressive in code style for any of these reasons (or more...?)
|
Assignment Submission: OO Ride Share
Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.
Reflection