Quin and Olga - Time by quinqu · Pull Request #15 · Ada-C13/oo-ride-share

quinqu · 2020-02-28T23:22:55Z

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 worked very well. It was nice to have a template to start out with
What inheritance relations exist between classes?	Driver, Trip, and Passenger were subclasses of csv record. TripDispatcher was it's own class
What composition relations exist between classes?	TripDispatcher has the composition relationship with all of the other classes.
Describe a decision you had to make when working on this project. What options were you considering? What helped you make your final decision?	Wave 4, implementing it and how to implement it. Time helped us make the final decision to go with working code, but not necessarily clean.
Give an example of a template method that you implemented for this assignment	self.from_csv
Give an example of a nominal test that you wrote for this assignment	for find_available_driver we tested if the function return the right driver as we tested for the correct driver ID.
Give an example of an edge case test that you wrote for this assignment	average rating was an edge case, we rounded the final rating to test for the correct answer
What is a concept that you gained more clarity on as you worked on this assignment	inheritance and how to override template methods.

beccaelenzil

Good work overall. It is clear that the learning goals around object composition and inheritance were met. You've done a great job working with this concepts and implementing some tricky logic. I've left some inline comments for you to review. This comments largely focus on writing tests that fully test nominal and edge cases for all of your methods. Please let me know if you have any questions and we can set up a group chat. Keep up the hard work.

beccaelenzil · 2020-03-10T16:56:31Z


+    def find_driver(id)
+      Driver.validate_id(id)
+      return @drivers.find { |driver| driver.id == id }


good use of an enumerable method.

beccaelenzil · 2020-03-10T17:01:07Z

+
+
+      total = @passenger.total_time_spent
+      expect(total).must_be_instance_of Float


It can be good to check that the return value is the correct datatype, but it is even more important to check that it is the correct value.

beccaelenzil · 2020-03-10T17:01:15Z

+
+
+      total = @passenger.net_expenditures
+      expect(total).must_be_instance_of Integer


It can be good to check that the return value is the correct datatype, but it is even more important to check that it is the correct value.

beccaelenzil · 2020-03-10T17:02:04Z

@@ -69,6 +70,65 @@
  end

  describe "net_expenditures" do


The tests for net_expenditures and total_time_spent should test the edge cases of a passenger having no trips an incomplete trips.

beccaelenzil · 2020-03-10T17:08:51Z

+      @trips << trip
+    end
+
+    def average_rating


Incomplete trips should not be including in the calculations of average_rating or total_revenue

beccaelenzil · 2020-03-10T17:10:07Z

+
+
+
+     describe "request trip" do


These tests look good! It looks like you implemented some of the optional conditions for choosing a driver. It is important to test these as well.

beccaelenzil · 2020-03-10T17:13:27Z

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`	These tests need to check that the correct value is computed.
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	Edge cases are not tested
Practices inheritence. `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	Each of these waves require lots of code and changes to different files. Practice make smaller commits, for instance after you implement the constructor method for each class.

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)	It appears that this functionality and tests are missing. Let me know if you have any questions.
There is reasonable test coverage for `TripDispatcher#request_trip`, and all tests pass	✔️

Overall Feedback

Overall Feedback	Criteria	yes/no
Yellow (Approaches Standards)	6+ in Code Review && 2+ in Functional Requirements	✔️

Code Style Bonus Awards

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

Quality	Yes?
Perfect Indentation	✅
Elegant/Clever	✅
Descriptive/Readable	✅
Concise	✅
Logical/Organized	✅

quinqu and others added 6 commits February 24, 2020 16:54

wave 1

a58d511

finished wave 2

3f9f6e6

wednes work

f129d51

thurs

16f6710

Finished wave 3 and tests

d2e247b

Finished wave 4 and tests

92c9591

beccaelenzil reviewed Mar 10, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Quin and Olga - Time #15

Quin and Olga - Time #15
quinqu wants to merge 6 commits into
Ada-C13:masterfrom
quinqu:master

quinqu commented Feb 28, 2020

Uh oh!

beccaelenzil left a comment

Uh oh!

beccaelenzil Mar 10, 2020

Uh oh!

beccaelenzil Mar 10, 2020

Uh oh!

beccaelenzil Mar 10, 2020

Uh oh!

beccaelenzil Mar 10, 2020

Uh oh!

beccaelenzil Mar 10, 2020

Uh oh!

beccaelenzil Mar 10, 2020

Uh oh!

beccaelenzil commented Mar 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants



		total = @passenger.total_time_spent
		expect(total).must_be_instance_of Float



		total = @passenger.net_expenditures
		expect(total).must_be_instance_of Integer

Uh oh!

Conversation

quinqu commented Feb 28, 2020

Assignment Submission: OO Ride Share

Reflection

Uh oh!

beccaelenzil left a comment

Choose a reason for hiding this comment

Uh oh!

beccaelenzil Mar 10, 2020

Choose a reason for hiding this comment

Uh oh!

beccaelenzil Mar 10, 2020

Choose a reason for hiding this comment

Uh oh!

beccaelenzil Mar 10, 2020

Choose a reason for hiding this comment

Uh oh!

beccaelenzil Mar 10, 2020

Choose a reason for hiding this comment

Uh oh!

beccaelenzil Mar 10, 2020

Choose a reason for hiding this comment

Uh oh!

beccaelenzil Mar 10, 2020

Choose a reason for hiding this comment

Uh oh!

beccaelenzil commented Mar 10, 2020

OO Ride Share

Major Learning Goals/Code Review

Testing Requirements

Overall Feedback

Code Style Bonus Awards

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants