Skip to content
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

oo-ride-share - Liz and Hannah #10

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

hertweckhr1
Copy link

OO Ride Share

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe a design decision you had to make when working on this project. What options were you considering? What helped you make your final decision? When we were working on wave 2, we were originally trying to make our code align with the spec files. We came to the decision to end up changing some of the spec files to fit the requirements, because the spec files were not aligned with changes to the code requirements. We were constantly hit with errors, and realized this was the right to make.
Describe any examples of composition that you encountered in this project, if any We had a user, class, driver class, and trip dispatcher. A passenger and driver can both have many trips. A trip can only have one passenger and one driver.
Describe the relationship between User and Driver Driver is a subclass of User. It inherits attributes from the user class, while also adding attributes of its own like driven_trips and status.
Describe a nominal test that you wrote for this assignment. We wrote a test to test if it assigns an available driver. It checks to make sure in the event of requesting a trip, the driver id must equal the expected value from the test file.
Describe an edge case test that you wrote for this assignment An edge case that we wrote, is if there are no available drivers, we then add an argument error.
Describe a concept that you/your pair gained more clarity on as you worked on this assignment
What are two discussion points that you and your pair discussed when giving/receiving feedback from each other that you would be willing to share? We realized that the code we're given may not be in perfect form, so it's wrong to make the assumption it is. Through that we learned more about deciphering errors, and editing spec files to help code run successfully. We also learned more about the use of inheritance and object oriented programming. As far as pair feedback - we both thought we knew when to go and ask when we did not understand. On a way to improve, we often had good ideas, but instead with trying one way, we ended up trying many different variations at the same time, and then code often became very confusing. In the future we think it would be beneficial to focus on one idea at a time.

@CheezItMan
Copy link

Ride Share

What We're Looking For

Feature Feedback
Baseline
Used Git Regularly Check, good number of commits and good commit messages, you used branches too!
Answer comprehension questions Check
Wave 1
Appropriate use of Ruby's Time Check
Trip has a helper method to calculate duration Check
User (passenger) has a method to calculate total cost of all trips Check
Tests for wave 1 Check
Wave 2
Driver inherits from User Check
Driver has add_driven_trip method Check
Driver has method to calculate average rating Check
Driver has method to calculate net expenditures and it uses super Check
Driver has a method to calculate total revenue Check
Tests for wave 2
Wave 3
TripDispatcher has a new method to create trips Check
creating a trip in TripDispatcher relies on methods in Driver and User (passenger) to modify their own attributes Check
Complex logic was correctly implemented Check
Tests for request_trip Check
Methods from wave 1 and 2 handle incomplete trips Check
Tests for wave 1 and 2 methods with incomplete trips Check
Wave 4 (Optional)
TripDispatcher now assigns trips to either the newest driver (no trips), or the driver who has not driven in the longest time Good logic for determining the driver. You are selecting a driver with no trips or a driver with the oldest trip.
Appropriate helper methods were made to help with complex logic Yes, although request_trip could be broken up further. You do have a great helper method in Driver.
Tests for wave 4 Good tests written for request_trip.
Overall You did really well. You have some great tests and solid application code. Great work!

passenger = find_passenger(user_id)

available_drivers = @drivers.find_all { |driver| driver.status == :AVAILABLE && driver.id != user_id}
raise ArgumentError, 'No available drivers' if available_drivers == []

Choose a reason for hiding this comment

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

Minor issue: Shouldn't be indented


def add_trip_in_progress(trip)
@status = :UNAVAILABLE
@driven_trips << trip

Choose a reason for hiding this comment

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

Why not call add_driven_trip instead?

end

def net_expenditures
return (super - total_revenue)

Choose a reason for hiding this comment

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

👍

end

# latest drive developed for wave 4
def latest_drive

Choose a reason for hiding this comment

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

Very clever method, you're not assuming the most recent trip is the last one in the array. That's good!

return @drivers.find { |driver| driver.id == id }
end

def request_trip(user_id)

Choose a reason for hiding this comment

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

This method is well done, but it's a bit long. You could break it into some helper methods doing the tasks of:

  1. Selecting an available driver
  2. Creating a new trip

@@ -58,5 +72,39 @@
expect(trip.passenger.id).must_equal 9
end
end

it 'calculates the net expenditure of all trips for the user' do

Choose a reason for hiding this comment

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

You should also test this with a person with no trips.

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