Space - Suely - Chelsea by ChelseaC13 · Pull Request #11 · Ada-C13/oo-ride-share

ChelseaC13 · 2020-02-28T21:29:17Z

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?	It was hard in the beginning. Figuring out what definitions were calling step by step helped. Going line by line and figuring out what is being asked was key.
What inheritance relations exist between classes?	there were multiple children to the parent CSV_record class with the exceptions of Trip_dispatcher.
What composition relations exist between classes?	We were able to replicate functions by just calling functions in the module. Like a Rideshare.
Describe a decision you had to make when working on this project. What options were you considering? What helped you make your final decision?	When is the best place/time to make a helper method? To solve this we just thought, would we need this function somewhere else? If yes then making a helper method would be a good idea
Give an example of a template method that you implemented for this assignment	We used self.from_csv(record) as a template method.
Give an example of a nominal test that you wrote for this assignment	Returns the total driver revenue and making sure it is numeric would be nominal.
Give an example of an edge case test that you wrote for this assignment	An edge case would be revenue for a trip below the trip fee. Example checking that a trip that costs $1.60 has no fee taken.
What is a concept that you gained more clarity on as you worked on this assignment	testing!

kaidamasaki · 2020-03-04T22:52:49Z

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`	✔️
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	✔️
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	✔️ (Some of your commit messages were a bit vague though, for example: "trying again" and "fixing errors". What are you trying again? What errors did you fix?)

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	✔️

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

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
Logical/Organized	✅

kaidamasaki

Good job! Here are some small things you can do to clean up your code.

kaidamasaki · 2020-03-03T19:14:16Z

+      @trips = trips || []
+
+      unless @vin.length == 17 && (@vin.is_a? String)
+        raise ArgumentError.new ('Your VIN is wrong.')


Including the bad arguments in your ArgumentError messages is helpful (also avoid spaces before argument lists):

Suggested change

raise ArgumentError.new ('Your VIN is wrong.')

raise ArgumentError.new('Your VIN (#{vin}) is wrong.')

kaidamasaki · 2020-03-03T20:10:24Z

+      approved_status = [:AVAILABLE, :UNAVAILABLE]
+
+      unless approved_status.include?(@status)
+        raise ArgumentError.new ('You must provide one of the following statuses :available, :unavailable')


Symbols are case sensitive:

Suggested change

raise ArgumentError.new ('You must provide one of the following statuses :available, :unavailable')

raise ArgumentError.new ('You must provide one of the following statuses :AVAILABLE, :UNAVAILABLE')

kaidamasaki · 2020-03-03T20:47:23Z

+
+    def average_rating
+      completed = @trips.select { |trip| trip.rating != nil }
+      completed.map {|trip| trip.rating}.sum.to_f / (completed.size == 0 ? 1 : completed.size)


You can use sum with a block to replace a usage of map. Good use of a ternary though. 😄

Suggested change

completed.map {|trip| trip.rating}.sum.to_f / (completed.size == 0 ? 1 : completed.size)

completed.sum {|trip| trip.rating}.to_f / (completed.size == 0 ? 1 : completed.size)

kaidamasaki · 2020-03-03T20:50:19Z

+      if start_time.class == String
+        start_time = Time.parse(start_time)
+      end


Since you're parsing times in from_csv you don't need to do that defensively here.

Suggested change

if start_time.class == String

start_time = Time.parse(start_time)

end

kaidamasaki · 2020-03-03T21:54:25Z

    end

+    def net_expenditures
+      completed = @trips.select { |trip| trip.cost != nil }


Nice use of select. 😃

kaidamasaki · 2020-03-03T23:03:35Z

+    before do
+
+      @passenger = RideShare::Passenger.new(
+        id: 9,
+        name: "Merl Glover III",
+        phone_number: "1-602-620-2330 x3723",
+        trips: []
+        )
+
+      driver = RideShare::Driver.new(
+        id: 54,
+        name: "Rogers Bartell IV",
+        vin: "1C9EVBRM0YBC564DZ",
+        status: :AVAILABLE      
+        )
+
+      @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
+        )
+
+      @trip2 = RideShare::Trip.new(
+        id: 10,
+        driver: driver,
+        passenger: @passenger,
+        start_time: Time.new(2016, 8, 11),
+        end_time: Time.new(2016, 8, 12),
+        cost: 40,
+        rating: 5
+        )
+
+      @trip3 = RideShare::Trip.new(
+        id: 11,
+        driver: driver,
+        passenger: @passenger,
+        start_time: Time.new(2016, 8, 11) 
+        )
+      end


Incorrect indentation:

Suggested change

before do

@passenger = RideShare::Passenger.new(

id: 9,

name: "Merl Glover III",

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

trips: []

)

driver = RideShare::Driver.new(

id: 54,

name: "Rogers Bartell IV",

vin: "1C9EVBRM0YBC564DZ",

status: :AVAILABLE

)

@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

)

@trip2 = RideShare::Trip.new(

id: 10,

driver: driver,

passenger: @passenger,

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

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

cost: 40,

rating: 5

)

@trip3 = RideShare::Trip.new(

id: 11,

driver: driver,

passenger: @passenger,

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

)

end

before do

@passenger = RideShare::Passenger.new(

id: 9,

name: "Merl Glover III",

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

trips: []

)

driver = RideShare::Driver.new(

id: 54,

name: "Rogers Bartell IV",

vin: "1C9EVBRM0YBC564DZ",

status: :AVAILABLE

)

@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

)

@trip2 = RideShare::Trip.new(

id: 10,

driver: driver,

passenger: @passenger,

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

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

cost: 40,

rating: 5

)

@trip3 = RideShare::Trip.new(

id: 11,

driver: driver,

passenger: @passenger,

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

)

end

kaidamasaki · 2020-03-03T23:05:50Z

+    before do
+      @passenger = RideShare::Passenger.new(
+        id: 9,
+        name: "Merl Glover III",
+        phone_number: "1-602-620-2330 x3723",
+        trips: []
+        )
+
+      driver = RideShare::Driver.new(
+        id: 54,
+        name: "Rogers Bartell IV",
+        vin: "1C9EVBRM0YBC564DZ",
+        status: :AVAILABLE      
+        )
+
+      @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
+        )
+
+      @trip2 = RideShare::Trip.new(
+        id: 10,
+        driver: driver,
+        passenger: @passenger,
+        start_time: Time.new(2016, 8, 11),
+        end_time: Time.new(2016, 8, 12),
+        cost: 40,
+        rating: 5
+        )
+
+        @trip3 = RideShare::Trip.new(
+        id: 11,
+        driver: driver,
+        passenger: @passenger,
+        start_time: Time.new(2016, 8, 11)
+        )
+
+      end


Incorrect indentation:

Suggested change

before do

@passenger = RideShare::Passenger.new(

id: 9,

name: "Merl Glover III",

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

trips: []

)

driver = RideShare::Driver.new(

id: 54,

name: "Rogers Bartell IV",

vin: "1C9EVBRM0YBC564DZ",

status: :AVAILABLE

)

@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

)

@trip2 = RideShare::Trip.new(

id: 10,

driver: driver,

passenger: @passenger,

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

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

cost: 40,

rating: 5

)

@trip3 = RideShare::Trip.new(

id: 11,

driver: driver,

passenger: @passenger,

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

)

end

before do

@passenger = RideShare::Passenger.new(

id: 9,

name: "Merl Glover III",

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

trips: []

)

driver = RideShare::Driver.new(

id: 54,

name: "Rogers Bartell IV",

vin: "1C9EVBRM0YBC564DZ",

status: :AVAILABLE

)

@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

)

@trip2 = RideShare::Trip.new(

id: 10,

driver: driver,

passenger: @passenger,

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

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

cost: 40,

rating: 5

)

@trip3 = RideShare::Trip.new(

id: 11,

driver: driver,

passenger: @passenger,

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

)

end

kaidamasaki · 2020-03-03T23:06:39Z

+      expect(@passenger.total_time_spent).must_equal 0
+      expect(@passenger.total_time_spent).must_be_kind_of Numeric 


These assertions are in the wrong order. If total_time_spent is 0 then it must already be Numeric:

Suggested change

expect(@passenger.total_time_spent).must_equal 0

expect(@passenger.total_time_spent).must_be_kind_of Numeric

expect(@passenger.total_time_spent).must_be_kind_of Numeric

expect(@passenger.total_time_spent).must_equal 0

kaidamasaki · 2020-03-04T18:17:48Z

+
+    it "calculates the duration of the trip corectly" do
+      expect(@trip.duration).must_be_kind_of Float
+      expect(@trip.duration).must_equal 1500.0


You should avoid using must_equal with Floats because they are imprecise. You should use must_be_close_to instead.

This requires that they be within 0.1:

Suggested change

expect(@trip.duration).must_equal 1500.0

expect(@trip.duration).must_be_close_to 1500.0, 0.1

kaidamasaki · 2020-03-04T22:07:56Z

+    def total_revenue
+      fee = 1.65
+      completed = @trips.select { |trip| trip.cost != nil }
+      return completed.map {|trip| trip.cost < fee ? 0 : trip.cost - fee}.sum * 0.8


You can also write this using reject (which might read a little clearer).

Suggested change

return completed.map {|trip| trip.cost < fee ? 0 : trip.cost - fee}.sum * 0.8

return completed.reject {|trip| trip.cost < fee}.sum { |trip| trip.cost - fee} * 0.8

kaidamasaki

Good job! Here are a somet things you can do to clean up your code.

SuelyBarreto and others added 20 commits February 24, 2020 18:10

gemfile 2.6.5

8372f9b

Wave one done! net_expenditures and total time spent

03964ac

wave 1

6aebc70

test commit

b1879f8

Stuck in the middle of wave 2

bba520c

trying again

a92f116

wave 2

fc6779d

not this version

7fc09d6

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

d3c9f0b

fixing errors

1d5265c

end of wave 2

9a86025

wave 2 done

1ffad39

chelsea's other version

e8845ad

EOD wednesday one error middle of wave 3

bb89c2d

wave 3 three more tests needed

fc019bc

wave 3 with more tests

1687f9e

skip removal

38ad6a6

Time.parse in Trip.from_csv

3afcbdf

optional wave 4

ad9df7c

Wave 4 done and cleaned up

6aacd3c

kaidamasaki reviewed Mar 4, 2020

View reviewed changes

	raise ArgumentError.new ('Your VIN is wrong.')
	raise ArgumentError.new('Your VIN (#{vin}) is wrong.')

	raise ArgumentError.new ('You must provide one of the following statuses :available, :unavailable')
	raise ArgumentError.new ('You must provide one of the following statuses :AVAILABLE, :UNAVAILABLE')

	completed.map {\|trip\| trip.rating}.sum.to_f / (completed.size == 0 ? 1 : completed.size)
	completed.sum {\|trip\| trip.rating}.to_f / (completed.size == 0 ? 1 : completed.size)

	if start_time.class == String
	start_time = Time.parse(start_time)
	end

		expect(@passenger.total_time_spent).must_equal 0
		expect(@passenger.total_time_spent).must_be_kind_of Numeric

	expect(@trip.duration).must_equal 1500.0
	expect(@trip.duration).must_be_close_to 1500.0, 0.1

	return completed.map {\|trip\| trip.cost < fee ? 0 : trip.cost - fee}.sum * 0.8
	return completed.reject {\|trip\| trip.cost < fee}.sum { \|trip\| trip.cost - fee} * 0.8

Uh oh!

Conversation

ChelseaC13 commented Feb 28, 2020

Assignment Submission: OO Ride Share

Reflection

Uh oh!

kaidamasaki commented Mar 4, 2020

OO Ride Share

Major Learning Goals/Code Review

Testing Requirements

Overall Feedback

Code Style Bonus Awards

Uh oh!

kaidamasaki left a comment

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!

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!

3 participants