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-maryam-kay #22

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

oo-ride-share-maryam-kay #22

wants to merge 53 commits into from

Conversation

kayxn23
Copy link

@kayxn23 kayxn23 commented Sep 1, 2018

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? We decided to create a trips array in the trip dispatcher category so we can have a list of trips that hav ever happened. Its useful to have that info in the Trip Dispatcher cus thats where we bring all the data together.
Describe any examples of composition that you encountered in this project, if any Driver inherited from User, and one of Driver's methods inherited from its super's same method.
Describe the relationship between User and Driver Driver inherits from User
Describe a nominal test that you wrote for this assignment. Making sure that the net_expenditures method returned a float
Describe an edge case test that you wrote for this assignment We tested for a negetive number to make sure it would pass.
Describe a concept that you/your pair gained more clarity on as you worked on this assignment Inheritance and debugging
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 discussed remaining calm during problems and be being patient for each other.

kayxn23 and others added 30 commits August 27, 2018 15:21
…sts creating valid Driver, valid VIN, and valid ID
marshi23 and others added 23 commits August 30, 2018 13:54
…exception in find_driver_by_availability method
@CheezItMan
Copy link

Ride Share

What We're Looking For

Feature Feedback
Baseline
Used Git Regularly Good number of commits
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, but you probably need one more test, see my note
Methods from wave 1 and 2 handle incomplete trips Check
Tests for wave 1 and 2 methods with incomplete trips NA
Overall Overall well done, you hit the learning goals. I did spot some issues and left notes in your code. Let me know if you have questions.


expect(driver).must_be_instance_of RideShare::Driver

expect(driver.trips).must_include trip

Choose a reason for hiding this comment

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

Not passing, the expectation should read:

expect(driver.driven_trips).must_include trip

Also see my note in TripDispatcher

}

trip = Trip.new(parsed_trip)
passenger.add_trip(trip)

Choose a reason for hiding this comment

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

You should also have:

driver.add_driven_trip(trip)

end
end
it "establishes the base data structures when instantiated" do
dispatcher = RideShare::TripDispatcher.new

Choose a reason for hiding this comment

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

Should have the files used for testing in the arguments to new

dispatcher = RideShare::TripDispatcher.new(USER_TEST_FILE,
        TRIP_TEST_FILE, DRIVER_TEST_FILE)

driven_trips.each do |trip|
all_ratings << trip.rating
end
return average = (all_ratings.sum / all_ratings.length).to_f.round(1)

Choose a reason for hiding this comment

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

You don't need average you could simply:

      return (all_ratings.sum / all_ratings.length).to_f.round(1)

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.

👍

expect(before_new_trip < after_new_trip).must_equal true
end

it "makes sure driver is not driving themself " do

Choose a reason for hiding this comment

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

I would add one test where there is one driver available and that driver requests a trip.

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