Space - Jeta and Stephanie by stephaniejmars · Pull Request #19 · Ada-C13/oo-ride-share

stephaniejmars · 2020-02-29T00:28:48Z

Assignment Submission: OO Ride Share

Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.

Reflection

Question	Answer
How did getting up to speed on the existing codebase go? If it went well, what worked? If it didn't go well, what will you do differently next time?	Having a set of instructions to find specific examples of things to look for and find out was very helpful to get familiar with the files.
What inheritance relations exist between classes?	Passenger, Driver, and Trip all inherit from their parent class CSV Record
What composition relations exist between classes?	Passenger to Trip, one to many. Driver to Trip is one to many. Driver to Passenger is many to many.
Describe a decision you had to make when working on this project. What options were you considering? What helped you make your final decision?	We were not sure if the trip cost was 1.65 or less, if the company kept the fee, or if the driver got the pay. We decided to give it to the company because typically that is was Corporate America would do, and we work for the company not the driver, but also, damn the man.
Give an example of a template method that you implemented for this assignment	self.build_path was a prebuilt template. It does not get called within the CSVRecord file, but other methods will call it.
Give an example of a nominal test that you wrote for this assignment	expect trip must be kind of trip
Give an example of an edge case test that you wrote for this assignment	if all drivers are unavailable
What is a concept that you gained more clarity on as you worked on this assignment	Classes, Inherited, calling files from other files, writing test.

…re start time.

…to test and reflect Driver class.

… updates status.

kaidamasaki · 2020-03-03T18:15:00Z

OO Ride Share

Major Learning Goals/Code Review

Criteria	yes/no, and optionally any details/lines of code to reference
The code demonstrates individual learning about `Time` and the responsibility of `Trip.from_csv`, and uses `Time.parse` in `Trip.from_csv`	Parsed `Time`s in `Trip.initialize`.
The code demonstrates breaking out complex logic in helper methods, such as making a helper method in `Trip` to calculate duration	✔️
There are tests for the nominal cases for the `Passenger#net_expenditures` and `Passenger#total_time_spent`	✔️
There is at least one edge case test for either `Passenger#net_expenditures` or `Passenger#total_time_spent` testing if the passenger has no trips	Missing.
Practices inheritance. `Driver` inherits from `CsvRecord`, and implements `from_csv`	✔️
Employs problem-solving and implements `Driver#average_rating` and `Driver#total_revenue`	✔️
Implements the `TripDispatcher#request_trip`, which creates an instance of `Trip` with a driver and passenger, adds the new trip to `@trips`, and changes the status of the driver	✔️
Practices composition. In `TripDispatcher#request_trip`, the driver gets connected to the new trip, the passenger gets connected to the new trip	✔️
Practices git with at least 10 small commits and meaningful commit messages	Your messages were fine but you should commit more often.

Testing Requirements

Testing Requirement	yes/no
There is reasonable test coverage for wave 1, and all wave 1 tests pass	✔️
There is reasonable test coverage for wave 2, and all wave 2 tests pass	✔️
Wave 3: Tests in wave 1 and wave 2 explicitly test that only completed trips should be calculated (and ignore in-progress trips)
There is reasonable test coverage for `TripDispatcher#request_trip`, and all tests pass	✔️ (You tested the nominal case in `before`, but testing a nominal case explicitly is best practices.)

Overall Feedback

Overall Feedback	Criteria	yes/no
Green (Meets/Exceeds Standards)	8+ in Code Review && 3+ in Functional Requirements
Yellow (Approaches Standards)	6+ in Code Review && 2+ in Functional Requirements	✔️
Red (Not at Standard)	0-5 in Code Review or 0,1 in Functional Reqs, or assignment is breaking/doesn’t run with less than 5 minutes of debugging

Additional Feedback

Generally this assignment was pretty reasonable. Your issues here were mostly around attention to detail. You got dinged for a lot of little things but overall you made reasonable decisions and your code was well organized.

This was awesome:

We decided to give it to the company because typically that is was Corporate America would do, and we work for the company not the driver, but also, damn the man.

Code Style Bonus Awards

Was the code particularly impressive in code style for any of these reasons (or more...?)

Quality	Yes?
Perfect Indentation	(See comments.)
Elegant/Clever
Descriptive/Readable	✅
Concise	(See comments.)
Logical/Organized	✅

kaidamasaki · 2020-03-03T00:34:55Z

+      raise ArgumentError if !valid_statuses.include?(@status)
+
+      raise ArgumentError if id <= 0 || id.nil?
+
+      raise ArgumentError if vin.length != 17


Remember, error messages are helpful for your users and for debugging.

Suggested change

raise ArgumentError if !valid_statuses.include?(@status)

raise ArgumentError if id <= 0 || id.nil?

raise ArgumentError if vin.length != 17

raise ArgumentError, "Bad status: #{status}" if !valid_statuses.include?(@status)

raise ArgumentError, "Bad id: #{id}" if id <= 0 || id.nil?

raise ArgumentError, "Bad vin: #{vin}" if vin.length != 17

kaidamasaki · 2020-03-03T00:37:57Z

+      total_ratings = 0
+      @trips.each do |trip|
+        total_ratings += trip.rating
+      end


You can simplify this using sum:

Suggested change

total_ratings = 0

@trips.each do |trip|

total_ratings += trip.rating

end

total_ratings = @trips.sum { |trip| trip.rating }

kaidamasaki · 2020-03-03T00:43:54Z

+      @trips.each do |trip|
+        if trip.cost <= 1.65
+          total_revenue += 0
+        else
+          trip_pay = trip.cost - 1.65
+          trip_pay = trip_pay * 0.80
+          total_revenue += trip_pay
+        end
+      end


Using each makes sense here. The logic is kind of complicated.

In case you're curious about the Enumerable version:

Suggested change

@trips.each do |trip|

if trip.cost <= 1.65

total_revenue += 0

else

trip_pay = trip.cost - 1.65

trip_pay = trip_pay * 0.80

total_revenue += trip_pay

end

end

total_revenue = @trips.select { |trip| trip.cost > 1.65 }.sum { |trip| (trip.cost - 1.65) * 0.8 }

kaidamasaki · 2020-03-03T00:46:17Z

+    def update_status 
+      @status = :UNAVAILABLE
+    end 


update_status is misleading since this method only ever sets @status to one value.

I would probably name this something like mark_unavailable or just omit this method entirely since you already have an attr_accessor on @status.

kaidamasaki · 2020-03-03T00:51:48Z


+
+      if driver
+        @driver = driver


Since you're only really using driver_id I wouldn't bother storing @driver as well since it could be confusing.

kaidamasaki · 2020-03-03T17:44:07Z

+      @driver = RideShare::Driver.new(
+        id: 7,
+        name: "Merl Glover Driver",
+        vin: "16026202330372367",
+        status: :AVAILABLE,
+        trips: []
+        )
+      @passenger = RideShare::Passenger.new(
+        id: 9,
+        name: "Merl Glover III",
+        phone_number: "1-602-620-2330 x3723",
+        trips: []
+        )
+
+        @trip1 = RideShare::Trip.new(
+        id: 8,
+        driver: @driver,
+        passenger: @passenger,
+        start_time: Time.new(2016, 8, 8),
+        end_time: Time.new(2016, 8, 9),
+        cost: 10,
+        rating: 5
+        )
+        @passenger.add_trip(@trip1)
+
+        @trip2 = RideShare::Trip.new(
+        id: 9,
+        driver: @driver,
+        passenger: @passenger,
+        start_time: Time.new(2016, 8, 11),
+        end_time: Time.new(2016, 8, 12),
+        cost: 11,
+        rating: 5
+        )
+        @passenger.add_trip(@trip2)


Incorrect indentation:

Suggested change

@driver = RideShare::Driver.new(

id: 7,

name: "Merl Glover Driver",

vin: "16026202330372367",

status: :AVAILABLE,

trips: []

)

@passenger = RideShare::Passenger.new(

id: 9,

name: "Merl Glover III",

phone_number: "1-602-620-2330 x3723",

trips: []

)

@trip1 = RideShare::Trip.new(

id: 8,

driver: @driver,

passenger: @passenger,

start_time: Time.new(2016, 8, 8),

end_time: Time.new(2016, 8, 9),

cost: 10,

rating: 5

)

@passenger.add_trip(@trip1)

@trip2 = RideShare::Trip.new(

id: 9,

driver: @driver,

passenger: @passenger,

start_time: Time.new(2016, 8, 11),

end_time: Time.new(2016, 8, 12),

cost: 11,

rating: 5

)

@passenger.add_trip(@trip2)

@driver = RideShare::Driver.new(

id: 7,

name: "Merl Glover Driver",

vin: "16026202330372367",

status: :AVAILABLE,

trips: []

)

@passenger = RideShare::Passenger.new(

id: 9,

name: "Merl Glover III",

phone_number: "1-602-620-2330 x3723",

trips: []

)

@trip1 = RideShare::Trip.new(

id: 8,

driver: @driver,

passenger: @passenger,

start_time: Time.new(2016, 8, 8),

end_time: Time.new(2016, 8, 9),

cost: 10,

rating: 5

)

@passenger.add_trip(@trip1)

@trip2 = RideShare::Trip.new(

id: 9,

driver: @driver,

passenger: @passenger,

start_time: Time.new(2016, 8, 11),

end_time: Time.new(2016, 8, 12),

cost: 11,

rating: 5

)

@passenger.add_trip(@trip2)

kaidamasaki · 2020-03-03T17:46:08Z

+      @driver = RideShare::Driver.new(
+        id: 7,
+        name: "Merl Glover Driver",
+        vin: "16026202330372367",
+        status: :AVAILABLE,
+        trips: []
+        )
+      @passenger = RideShare::Passenger.new(
+        id: 9,
+        name: "Merl Glover III",
+        phone_number: "1-602-620-2330 x3723",
+        trips: []
+        )
+
+        @trip1 = RideShare::Trip.new(
+        id: 8,
+        driver: @driver,
+        passenger: @passenger,
+        start_time: Time.new(2016, 8, 8, 21, 00, 00),
+        end_time: Time.new(2016, 8, 8, 21, 10, 00),
+        cost: 10,
+        rating: 5
+        )
+        @passenger.add_trip(@trip1)
+
+        @trip2 = RideShare::Trip.new(
+        id: 9,
+        driver: @driver,
+        passenger: @passenger,
+        start_time: Time.new(2016, 8, 11, 20, 00, 00),
+        end_time: Time.new(2016, 8, 11, 20, 8, 00),
+        cost: 11,
+        rating: 5
+        )
+        @passenger.add_trip(@trip2)


Incorrect indentation (see above).

Also if you'd like to share a before block between two describe blocks you can move it up to their parent describe block. (Or create a new parent describe block.)

kaidamasaki · 2020-03-03T17:46:43Z


      expect(dispatcher.trips).must_be_kind_of Array
      expect(dispatcher.passengers).must_be_kind_of Array
-      # expect(dispatcher.drivers).must_be_kind_of Array


I'm not clear on why you deleted this. The provided tests are part of the requirements.

kaidamasaki · 2020-03-03T17:48:56Z

+      expect(@trip.end_time).must_be_nil
+      expect(@trip.cost).must_be_nil
+      expect(@trip.rating).must_be_nil


Good use of must_be_nil!

kaidamasaki · 2020-03-03T17:49:32Z

+      @dispatcher.drivers.each do |driver|
+           driver.status = :UNAVAILABLE
+      end
+      expect{@dispatcher.request_trip(3)}.must_raise ArgumentError


Adding some spacing can make this read a little clearer.

Suggested change

expect{@dispatcher.request_trip(3)}.must_raise ArgumentError

expect { @dispatcher.request_trip(3) }.must_raise ArgumentError

kaidamasaki

Most of these are pretty minor things that will help going forward.

stephaniejmars added 8 commits February 25, 2020 15:57

added time parse to trips, and raise argument error for end time befo…

bb1a110

…re start time.

wave 1 completed for trips. trip druation and test working.

4d8ba26

added net expenditures

0dcfd67

total time spent method added.

e663cd8

Wave 2 completed, driver class created, and all other files modified …

459bcdb

…to test and reflect Driver class.

fixed driver error

6b027e4

updated trip dispatcher to build new trips. adds to drivers trips and…

c4bef89

… updates status.

final updates

a456630

kaidamasaki reviewed Mar 3, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Space - Jeta and Stephanie#19

Space - Jeta and Stephanie#19
stephaniejmars wants to merge 8 commits intoAda-C13:masterfrom
stephaniejmars:master

stephaniejmars commented Feb 29, 2020

Uh oh!

kaidamasaki commented Mar 3, 2020 •

edited

Loading

Uh oh!

kaidamasaki Mar 3, 2020

Uh oh!

kaidamasaki Mar 3, 2020

Uh oh!

kaidamasaki Mar 3, 2020

Uh oh!

kaidamasaki Mar 3, 2020

Uh oh!

kaidamasaki Mar 3, 2020

Uh oh!

kaidamasaki Mar 3, 2020

Uh oh!

kaidamasaki Mar 3, 2020

Uh oh!

kaidamasaki Mar 3, 2020

Uh oh!

kaidamasaki Mar 3, 2020

Uh oh!

kaidamasaki Mar 3, 2020

Uh oh!

kaidamasaki left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	expect{@dispatcher.request_trip(3)}.must_raise ArgumentError
	expect { @dispatcher.request_trip(3) }.must_raise ArgumentError

Conversation

stephaniejmars commented Feb 29, 2020

Assignment Submission: OO Ride Share

Reflection

Uh oh!

kaidamasaki commented Mar 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

OO Ride Share

Major Learning Goals/Code Review

Testing Requirements

Overall Feedback

Additional Feedback

Code Style Bonus Awards

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaidamasaki left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kaidamasaki commented Mar 3, 2020 •

edited

Loading