Sigrid & Barbara, Edges, oo-ride-share #7

BarbaraWidjono · 2018-08-31T23:00:02Z

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 needed to make a design decision as to how an 'InProgress' trip was to be handled by the constructor and subsequent calculations. Two options considered: changing the falsey nil into a string "nil"--the other option was changing the falsey nil into a string "InProgress." It was decided to convert falsey nil into string "InProgress" to prevent any confusion in testing between falsey nil and string "nil."
Describe any examples of composition that you encountered in this project, if any	Yes, a single TripDispatcher object can have multiple Users, Drivers and Trips. A single Driver can have many Trips. A single User can have many Trips.
Describe the relationship between `User` and `Driver`	The class User is the superclass to the subclass Driver.
Describe a nominal test that you wrote for this assignment.	Within the 'request_a_trip' test, we checked for the instantiation of a Trip instance and checked that the new instance was added to the user, driver and all trips.
Describe an edge case test that you wrote for this assignment	Within the 'request_a_trip' test, we checked that a trip could not be requested if the user_id was invalid (was not a User object).
Describe a concept that you/your pair gained more clarity on as you worked on this assignment	Gained clarity on how a variable points to a memory location which stores a value. A little bit more clarity (for Barb) regarding Minitests and the use of 'before-do' blocks.
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?	I appreciated Sigrid's attention to detail and her ability to write and explain testing. I (Sigrid) appreciated Barb's checking in with me on how I felt about the division of work, and her willingness to work both individually and collaboratively.

… check in Trip#initialize to check for end time prior to start time.

…uding names and phone numbers.

…TripDispatcher file.

…t. Also corrected driver.add_driven_trip and passenger_as_driver.add_trip lines in load_trips method.

…n method in trip.rb file to handle @end_time value of In Progress.

…lass to ignore trips In Progress.

…s to ignore In Progress trips.

droberts-sea · 2018-09-08T01:57:27Z

Ride Share

What We're Looking For

Feature	Feedback
Baseline
Used Git Regularly	yes
Answer comprehension questions	yes
Wave 1
Appropriate use of Ruby's `Date`	yes
`Trip` has a helper method to calculate duration	yes
`User` (passenger) has a method to calculate total cost of all trips	yes
Tests for wave 1	yes
Wave 2
`Driver` inherits from `User`	yes
`Driver` has `add_driven_trip` method	yes
`Driver` has method to calculate average rating	yes
`Driver` has method to calculate net expenditures and it uses `super`	yes
`Driver` has a method to calculate total revenue	yes
Tests for wave 2	yes - missing some edge cases, see inline
Wave 3
`TripDispatcher` has a new method to create trips	yes
creating a trip in `TripDispatcher` relies on methods in `Driver` and `User` (passenger) to modify their own attributes	yes
Complex logic was correctly implemented	yes
Tests for `request_trip`	yes, good work
Methods from wave 1 and 2 handle incomplete trips	yes - good work
Tests for wave 1 and 2 methods with incomplete trips	yes
Overall	Great job overall! This submission is quite strong. There are a few places where things could be cleaned up or test coverage expanded, which I've tried to call out inline below, but in general I'm happy with what I see. Keep up the hard work!

droberts-sea · 2018-09-08T01:48:12Z

lib/trip.rb

+      if @end_time == nil
+        @end_time = "In Progress"
+      elsif @end_time < @start_time
+        raise ArgumentError, "End time is before start time"


I would much rather have nil than a string to indicate a trip is in progress.

droberts-sea · 2018-09-08T01:49:47Z

specs/user_spec.rb

+    it "Does not include trips in progress in calculation of total amount spent on trips" do
+
+      run_trip_dispatcher = RideShare::TripDispatcher.new()
+        trip2 = RideShare::Trip.new(id: 9, driver: nil, passenger: @user,


Good work getting this edge case. Another one to consider: what if the user has not yet taken a trip?

droberts-sea · 2018-09-08T01:50:07Z

lib/driver.rb

+    def net_expenditures
+     net_expenditures = super
+     difference = net_expenditures - total_revenue
+     # difference = super - total_revenue # can use super in place of above code.


Good use of super here.

droberts-sea · 2018-09-08T01:50:58Z

lib/driver.rb

+      @driven_trips.each do |x|
+        # binding.pry
+        if x.rating == "In Progress"
+          next


You tended to use .each iteration to calculate totals/sum things in Driver and User, even though things like .sum is available. Honestly, I don't believe that you ALWAYS need to use fancy Enumerable methods like .sum over .each iteration every time, but I want to make sure you know that those are possibilities.

droberts-sea · 2018-09-08T01:52:56Z

specs/driver_spec.rb

+      it 'checks if driver.net_expenditures overrides user.net_expenditures' do
+      difference = @driver.net_expenditures
+      expect(difference).must_equal 10 - ((4 - 1.65) * 0.80)
+      end


This is a good start, but I believe there are some more interesting cases here. For example, what if a driver has:

No driven trips

No trips as a passenger

An incomplete trip as a driver or passenger

Made more money than they've spent

droberts-sea · 2018-09-08T01:53:47Z

lib/trip_dispatcher.rb

+
+     else
+       return "No available drivers"
+       # return nil


You should either return nil or raise an exception here, not return a string. There are a number of reasons for this, but ultimately what it comes down to is, in order to check that a string contains an error message, you need to read it.

droberts-sea · 2018-09-08T01:55:05Z

lib/trip_dispatcher.rb

+     # Checking for available drivers that do not have the same id as the user requesting a ride
+     available_drivers = []
+     @drivers.each do |x|
+       if x.status == :AVAILABLE && x.id != user_id.to_i


This would be a great piece of logic to move to a helper method.

sdbenezra and others added 28 commits August 27, 2018 15:48

Modify TripDispatcher#load_trips to store time as Time instances. Add…

e9c2e0d

… check in Trip#initialize to check for end time prior to start time.

Correct data in trip_test.csv

114fd9f

Corrected csv data from Slack

0700e0a

Merge branch 'master' of https://github.com/BarbaraWidjono/oo-ride-share

1a70fb3

Added .duration method

19523c5

Added test: Calc duration of trip

8ebdc03

Start work on total expenditures

f9c8df4

Complete net_expenditures method in user.rb file

50e6f8b

Complete test for net_expenditures method in specs/user_spec.rb file

6f47520

Removed comments

48090a3

Fix merge conflict

ee2b789

Added total_time_spent method and test

251e6d9

Added class Driver

69519d0

Working on driver class & specs.

8c0c924

TripDispatcher class loads drivers without name and phone number.

1928564

TripDispatcher now loads drivers and creates driver object array incl…

aad1567

…uding names and phone numbers.

Replace user with driver in passenger array in load_driver method of …

e4a9ad6

…TripDispatcher file.

Added add_driven_trip method to driver class.

bdb8835

Adding average_rating method to class Driver

cc62f35

Added total_revenue method to driver class.

fb16ed7

Added Driver.net_expenditures method

1ca4aac

Finalize driver add driven trip and refactoring.

629ee67

Add request_trip method in trip_dispatcher class and request_trip tes…

40ff18a

…t. Also corrected driver.add_driven_trip and passenger_as_driver.add_trip lines in load_trips method.

Update trip.rb to correctly handle @cost value of nil. Change duratio…

85fc892

…n method in trip.rb file to handle @end_time value of In Progress.

Change calculation of net_expenditures and total_time_spent in user c…

9f90936

…lass to ignore trips In Progress.

Change calculation of average rating and total revenue in driver clas…

84b06e7

…s to ignore In Progress trips.

Refactor to remove unused variables.

2f5780a

Added argument_error if an invalid user is requesting a ride

3f388c6

droberts-sea reviewed Sep 8, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sigrid & Barbara, Edges, oo-ride-share #7

Sigrid & Barbara, Edges, oo-ride-share #7

BarbaraWidjono commented Aug 31, 2018

droberts-sea commented Sep 8, 2018

droberts-sea Sep 8, 2018

droberts-sea Sep 8, 2018

droberts-sea Sep 8, 2018

droberts-sea Sep 8, 2018

droberts-sea Sep 8, 2018

droberts-sea Sep 8, 2018

droberts-sea Sep 8, 2018

Sigrid & Barbara, Edges, oo-ride-share #7

Are you sure you want to change the base?

Sigrid & Barbara, Edges, oo-ride-share #7

Conversation

BarbaraWidjono commented Aug 31, 2018

OO Ride Share

Comprehension Questions

droberts-sea commented Sep 8, 2018

Ride Share

What We're Looking For

droberts-sea Sep 8, 2018

Choose a reason for hiding this comment

droberts-sea Sep 8, 2018

Choose a reason for hiding this comment

droberts-sea Sep 8, 2018

Choose a reason for hiding this comment

droberts-sea Sep 8, 2018

Choose a reason for hiding this comment

droberts-sea Sep 8, 2018

Choose a reason for hiding this comment

droberts-sea Sep 8, 2018

Choose a reason for hiding this comment

droberts-sea Sep 8, 2018

Choose a reason for hiding this comment