Space - Shonda & Diana by dnguye2 · Pull Request #9 · Ada-C13/oo-ride-share

dnguye2 · 2020-02-28T21:03:21Z

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?	The first day, it was very overwhelming to understand what was going on. Using pry helped to understand how the codebase worked. Next time, it would be helpful to look at every file and also examine the tests. This would have expedited understanding of the codebase.
What inheritance relations exist between classes?	Within the trip dispatcher, which is the holder, csv_record was the super class of the following sub classes which had inheritance from csv_record: drivers, passengers, and trips.
What composition relations exist between classes?	The driver class and the passenger class has a trip.
Describe a decision you had to make when working on this project. What options were you considering? What helped you make your final decision?	For the net expenditures method, we had to handle when the @trips array was nil. We were trying to decide to use a `nil?` which didn't work at first. Then we realized we had use `.empty?` because `nil?` refers nothing whereas @trips had an empty array which is not nil. This led us to realize `.empty?` was the best choice because it covered the case we were looking for.
Give an example of a template method that you implemented for this assignment	For the driver class, the template method we created and used was `def self.from_csv(record)`.
Give an example of a nominal test that you wrote for this assignment	For the passenger `net_expenditures` method, we wrote a nominal test for `it "returns the correct total amount of money that a passenger has spent"`
Give an example of an edge case test that you wrote for this assignment	For the driver `average_rating` we wrote an edge case test for `it "returns zero if no driven trips" do` and `it "returns 0 if trip costs <= $1.65" do`
What is a concept that you gained more clarity on as you worked on this assignment	We got more clarity on how classes function in relation to each other. And as a whole, object oriented programming and design. In addition, we gained more clarity on handling nil cases.

kaidamasaki · 2020-03-06T01:32:31Z

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`	Somewhat.
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	✔️

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 tips for how you can clean up your code. 😄

kaidamasaki · 2020-03-05T22:21:16Z

lib/driver.rb

+    @status = status.to_sym
+    @trips = trips || []
+
+    vin_pattern = /^[A-Z0-9]{17}$/i


Regular expressions! 🎉

kaidamasaki · 2020-03-05T22:21:50Z

lib/driver.rb

+
+    vin_pattern = /^[A-Z0-9]{17}$/i
+    if (vin_pattern =~ @vin) == nil
+      raise ArgumentError, "Invalid VIN"


It's helpful to include the bad argument in the message for your ArgumentErrors:

Suggested change

raise ArgumentError, "Invalid VIN"

raise ArgumentError, "Invalid VIN: #{@vin}"

kaidamasaki · 2020-03-05T22:22:44Z

lib/driver.rb

+    @trips << trip
+  end
+
+  def change_status


The name change_status is misleading since it just sets the status to :UNAVAILABLE.

Suggested change

def change_status

def make_unavailable

kaidamasaki · 2020-03-05T22:24:15Z

lib/driver.rb

+        else
+        (trip.rating).to_f
+        end


Incorrect indentation:

Suggested change

else

(trip.rating).to_f

end

else

(trip.rating).to_f

end

kaidamasaki · 2020-03-05T22:26:45Z

lib/driver.rb

+      cost = ((average_rating).inject(:+))/((average_rating.length) - nil_cases)
+      return cost


You have a lot of extra parenthesis and can directly return the result of this computation, also inject(:+) is the same as sum):

Suggested change

cost = ((average_rating).inject(:+))/((average_rating.length) - nil_cases)

return cost

return average_rating.sum / (average_rating.length - nil_cases)

Remember, . has higher precedence than arithmetic.

kaidamasaki · 2020-03-06T01:10:55Z

test/passenger_test.rb

+      @passenger = RideShare::Passenger.new(
+        id: 9,
+        name: "Merl Glover III",
+        phone_number: "1-602-620-2330 x3723",
+        trips: []
+        )


Incorrect indentation:

Suggested change

@passenger = RideShare::Passenger.new(

id: 9,

name: "Merl Glover III",

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

trips: []

)

@passenger = RideShare::Passenger.new(

id: 9,

name: "Merl Glover III",

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

trips: []

)

kaidamasaki · 2020-03-06T01:13:39Z

lib/trip.rb

+
+    # 1.1 #4 instance method of duration of trip
+    def duration
+      return (Time.parse(@end_time) - Time.parse(@start_time)).to_i


You're already parsing the time in from_csv so this is redundant.

kaidamasaki · 2020-03-06T01:19:01Z

test/passenger_test.rb

+          start_time: "#{Time.now - 60 * 60}",
+          end_time: "#{Time.now + 60 * 60}",


You should just pass Time objects into the test.

Suggested change

start_time: "#{Time.now - 60 * 60}",

end_time: "#{Time.now + 60 * 60}",

start_time: Time.now - 60 * 60,

end_time: Time.now + 60 * 60,

kaidamasaki · 2020-03-06T01:29:53Z

lib/trip_dispatcher.rb

+
+      # handles case when there are no available drivers
+      if @drivers.find {|driver| driver.status == :AVAILABLE}.nil?
+        raise ArgumentError.new("No available drivers.")


This isn't an issue with the arguments here. You should use a custom error class here.

For example at the top of the file you can define:

class NoDriverError < StandardError end

And then:

Suggested change

raise ArgumentError.new("No available drivers.")

raise NoDriverError.new("No available drivers.")

kaidamasaki · 2020-03-06T01:30:47Z

lib/trip_dispatcher.rb

+      @driver.change_status
+
+      @trip_data = {
+        id: @trips.last.id + 1,


Nice use of last here to assign a unique id.

dnguye2 added 15 commits February 25, 2020 14:37

completed step 3 of wave 1.1

efdffbf

completed wave 1.1 code

50380a1

Wave 1.2 net_expenditures code completed

854b2e0

Wave 1.2.2 -Completed total_time_spent method

8eaacc5

Wave 1.2 edge case tests completed

9c8f17b

Wave 2 driver class and passed instantiation tests

0be429b

Wave 2 updated trip+tests w/ driver id and driver

67365b1

Wave 2 loaded drivers

21d070c

Wave 2 completed, added driver methods and tests

8da28f1

require_relative driver for driver tests

b3d3a7d

cleaned up files

85c57ae

wave 3 updated tests and cleaned up code

889a7a0

Wave 3 half of request_trip method code completed

3787bdf

Wave 3 code completed

87a60e2

final commit

b67eb7c

kaidamasaki reviewed Mar 6, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Space - Shonda & Diana#9

Space - Shonda & Diana#9
dnguye2 wants to merge 15 commits intoAda-C13:masterfrom
dnguye2:master

dnguye2 commented Feb 28, 2020

Uh oh!

kaidamasaki commented Mar 6, 2020

Uh oh!

kaidamasaki left a comment

Uh oh!

kaidamasaki Mar 5, 2020

Uh oh!

kaidamasaki Mar 5, 2020

Uh oh!

kaidamasaki Mar 5, 2020

Uh oh!

kaidamasaki Mar 5, 2020

Uh oh!

kaidamasaki Mar 5, 2020

Uh oh!

kaidamasaki Mar 6, 2020

Uh oh!

kaidamasaki Mar 6, 2020

Uh oh!

kaidamasaki Mar 6, 2020

Uh oh!

kaidamasaki Mar 6, 2020

Uh oh!

kaidamasaki Mar 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	raise ArgumentError, "Invalid VIN"
	raise ArgumentError, "Invalid VIN: #{@vin}"

		cost = ((average_rating).inject(:+))/((average_rating.length) - nil_cases)
		return cost

	cost = ((average_rating).inject(:+))/((average_rating.length) - nil_cases)
	return cost
	return average_rating.sum / (average_rating.length - nil_cases)

		start_time: "#{Time.now - 60 * 60}",
		end_time: "#{Time.now + 60 * 60}",

	raise ArgumentError.new("No available drivers.")
	raise NoDriverError.new("No available drivers.")

Conversation

dnguye2 commented Feb 28, 2020

Assignment Submission: OO Ride Share

Reflection

Uh oh!

kaidamasaki commented Mar 6, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants