Kat F. & Leanne R. Ride Share OOD #12

leannerivera · 2018-09-01T00:01:38Z

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?	putting new classes in separate files, making sure that the lines are in correct order...
Describe any examples of composition that you encountered in this project, if any	Putting the classes in the modules as a namespace is an example of composition
Describe the relationship between `User` and `Driver`	Driver is a subclass of User
Describe a nominal test that you wrote for this assignment.	Making sure that new Driver is an instance of Driver class
Describe an edge case test that you wrote for this assignment	making sure that VIn is no less or greater than 17 characters
Describe a concept that you/your pair gained more clarity on as you worked on this assignment	spec tests
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?

leannerivera · 2018-09-04T16:48:23Z

Our recent commits aren't showing up though we had pushed them multiple times...

CheezItMan · 2018-09-06T17:31:25Z

@leannerivera Come by and show me what's happening and we can look for your missing commits.

CheezItMan · 2018-09-06T21:02:34Z

Ride Share

What We're Looking For

Feature	Feedback
Baseline
Used Git Regularly	Good number of commits, and mostly good commit messages, you don't need to reference waves in the commit messages. Instead just describe functionality added.
Answer comprehension questions	Check
Wave 1
Appropriate use of Ruby's `Time`	Check
`Trip` has a helper method to calculate duration	Done, but awkwardly so. See my inline notes.
`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	Yes, but see my notes regarding wave 3
`Driver` has method to calculate net expenditures and it uses `super`	It's there with a weird class variable when it could use `super`
`Driver` has a method to calculate total revenue	Check, but see my notes for wave 3.
Tests for wave 2	Some broken tests, instances of Trip missing `end_time` or `start_time`.
Wave 3
`TripDispatcher` has a new method to create trips	The method i started, but not complete
creating a trip in `TripDispatcher` relies on methods in `Driver` and `User` (passenger) to modify their own attributes	Check
Complex logic was correctly implemented	Nope
Tests for `request_trip`	You do have some tests here
Methods from wave 1 and 2 handle incomplete trips	Nope
Tests for wave 1 and 2 methods with incomplete trips	Nope
Overall	There's a lot not working here. Mostly tests that were already broken not corrected. You also have some methods with bugs or errors. I have left notes inline in your code. You definitely need more practice with the nested data structures.

CheezItMan · 2018-09-06T20:35:17Z

lib/driver.rb

+    def average_rating
+      ratings = 0
+      count = 0
+      @driven_trips.each do |trip|


You should have something added to deal with incomplete trips

completed_trips = @driven_trips.select {|trip| trip.end_time != nil }

And then only use the completed_trips.

The same concept can apply to average_rating, total_revenue etc

CheezItMan · 2018-09-06T20:37:31Z

lib/driver.rb

+        count += 1
+      end
+
+      if ratings = 0 || count = 0


Look at this if statement.

You're using = not ==

This doesn't seem to serve much of a purpose.

CheezItMan · 2018-09-06T20:39:20Z

lib/driver.rb

+
+        total_revenue += (trip.cost - 1.65)
+      end
+      return (total_revenue/ 0.8).round(2)


You're increasing the total revenue by 1.25 times?

CheezItMan · 2018-09-06T20:40:58Z

lib/driver.rb

+    def net_expenditures
+      trip_costs = 0
+      # driver/passenger trip cost - total_revenue
+      if TripDispatcher.trips.passenger == self.id


What is this doing?

You can use the superclass method in this:

return super - self.total_revenue

CheezItMan · 2018-09-06T20:41:53Z

lib/trip.rb

-      "PassengerID=#{passenger&.id.inspect}>"
+
+    def trip_to_seconds
+      midnight = Time.local(2018,8,26,0,0,0)


This could also be done as:

return end_time - start_time

CheezItMan · 2018-09-06T20:57:03Z

lib/user.rb

+
+    def net_expenditures
+      total = 0
+      self.trips.each do |trip|


For wave 3 you would need to adapt this to incomplete trips.

completed_trips = self.trips.select { |trip| trip.end_time != nil }

CheezItMan · 2018-09-06T20:57:52Z

lib/user.rb

+    end
+
+def total_time_spent
+  @passenger.trips.each do |trip|


For wave 3 you would also need to filter the list for only completed trips

CheezItMan · 2018-09-06T20:58:21Z

lib/user.rb

+  @passenger.trips.each do |trip|
+    t1 = Time.parse(trip.start_time)
+    t2 = Time.parse(trip.end_time)
+    duration += sprintf("%0.02f", (t2 - t1) % 60)


This should return a number, not a string.

CheezItMan · 2018-09-06T20:59:28Z

specs/trip_dispatch_spec.rb

+
+  it "changes driver status to unavailable" do
+    request = @dispatcher.request_trip(6)
+    expect(request.driver.status).must_equal :UNAVAILABLE


You should also verify the driver was available before the request_trip

CheezItMan · 2018-09-06T20:59:53Z

specs/trip_dispatch_spec.rb

+  end
+
+  it "raises an error if there are no available drivers" do
+    dispatcher = RideShare::TripDispatcher.new(USER_TEST_FILE,


leannerivera and others added 29 commits August 27, 2018 14:35

Require 'time'

80a1dab

time parsed?

aabcdf9

wave 1 invalid is invalid

5b50194

for pull

f26c9de

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

6f94786

can duration in seconds

748500f

net expend test need method

b049149

net expenditure method passed, can net expend.

1be92cf

added user ride duration and passed

b5203c2

Driver class created, tests passed.

f536ff4

wave 2 driver instance started

668f408

load driver method start

9f0ebc7

working on trip dispathcher, fixed specs

b3f6eec

load drivers method done WAVE 2 PASSED

6432766

Tried load_drivers method will add_trips first

424d6f0

troubleshooting tripdispatcher

5393445

edited load driver method

f9d9bf8

add_drive_trips

76a09d2

can driver average rating

14b0794

total_revenue complete

706fbfa

start with request trip with user id

e1ab941

wave 3 request trip method

e37acfe

wave 3 new trip test complete errors expected rate = nil

e3d71bd

can request trip, pass

76c81c4

request trip - can driver status change to unavailable

de19c67

request trip: driver cannot drive themselves true

c81a8ca

error when no driver available - pass

c6c57cf

minor

cdda244

all methods for wave 3 added

aa8a959

CheezItMan reviewed Sep 6, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kat F. & Leanne R. Ride Share OOD #12

Kat F. & Leanne R. Ride Share OOD #12

leannerivera commented Sep 1, 2018

leannerivera commented Sep 4, 2018

CheezItMan commented Sep 6, 2018

CheezItMan commented Sep 6, 2018

CheezItMan Sep 6, 2018

CheezItMan Sep 6, 2018

CheezItMan Sep 6, 2018

CheezItMan Sep 6, 2018

CheezItMan Sep 6, 2018

CheezItMan Sep 6, 2018

CheezItMan Sep 6, 2018

CheezItMan Sep 6, 2018

CheezItMan Sep 6, 2018

CheezItMan Sep 6, 2018

Kat F. & Leanne R. Ride Share OOD #12

Are you sure you want to change the base?

Kat F. & Leanne R. Ride Share OOD #12

Conversation

leannerivera commented Sep 1, 2018

OO Ride Share

Comprehension Questions

leannerivera commented Sep 4, 2018

CheezItMan commented Sep 6, 2018

CheezItMan commented Sep 6, 2018

Ride Share

What We're Looking For

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment