Skip to content

Time - Hannah J & Vera#8

Open
veralizeth wants to merge 11 commits intoAda-C13:masterfrom
veralizeth:master
Open

Time - Hannah J & Vera#8
veralizeth wants to merge 11 commits intoAda-C13:masterfrom
veralizeth:master

Conversation

@veralizeth
Copy link

@veralizeth veralizeth commented Feb 28, 2020

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? We think it went well. What we did was to read first and make diagrams. From there, we tried to go through step by step depending the wave. We are so glad that we did this project as a pair.
What inheritance relations exist between classes? We have a super class, CsvRecord and we have subclasses, Passenger, Driver and Trip.
What composition relations exist between classes? Passenger and Trip have one to many relationship. Driver and Trip also have one to many relationship. TripDispatcher and Driver & TripDispatcher and Passenger & TripDispatcher and Trip have also one to many relationships.
Describe a decision you had to make when working on this project. What options were you considering? What helped you make your final decision? During the Wave 4 part, we needed to figure out how to sort available_drivers array with sorted trips inside of the available drivers. We made a decision based on the test results by using "puts" and rubber ducking each other.
Give an example of a template method that you implemented for this assignment We have a template method, self.from_csv in CsvRecord class. We implemented this method in Driver & Passenger & Trip class
Give an example of a nominal test that you wrote for this assignment For the trip_dispatch_test, we wrote a nominal test called "Selects an available driver and switch to an unavailable status" to make sure we have the correct status for each driver.
Give an example of an edge case test that you wrote for this assignment For the driver_test, we wrote a edge case called "Returns the correct total revenue when the cost of a trip was less that $1.65" to handle the total revenue calculating the 80% of trip cost without subtracting the fee ($1.65)
What is a concept that you gained more clarity on as you worked on this assignment By doing this project, we understood the concepts of Inheritance (super class and subclasses) and object compositions. It was very helpful for us to have diagrams to understand the (is a / has a) relationships . We were very glad to see the useful template methods, and learn when to use helper methods. Furthermore, we gained more clarity on how to use before blocks, nominal and edge cases in test files.
Collaps

@CheezItMan
Copy link

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

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

Code Style Bonus Awards

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

Quality Yes?
Perfect Indentation
Descriptive/Readable
Logical/Organized

Summary

Nice work, you hit all the learning goals here. Nice work on waves 1-3 especially the tests.

@trips.each do |trip|
total_rating += trip.rating
end
return (total_rating / @trips.length).round(2)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return (total_rating / @trips.length).round(2)
return (total_rating / @trips.length.to_f ).round(2)

if trip.cost >= 1.65
total_revenue += ((trip.cost - 1.65) * 0.80)
# QUESTION
elsif trip.cost < 1.65

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need the elsif since this is the exact opposite of the if.

Suggested change
elsif trip.cost < 1.65
else

Comment on lines +87 to +88
start_time: Time.parse(record[:start_time]),
end_time: Time.parse(record[:end_time]),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


def request_trip(passenger_id)

available_driver = find_frist_driver

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spelling? frist?

Suggested change
available_driver = find_frist_driver
available_driver = find_first_driver



# Helper method
def find_frist_driver

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def find_frist_driver
def find_first_driver


it "Returns total revenue across all their trips" do
#Assert
expect(@driver.total_revenue).must_be_close_to 9.28, 0.01

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good use of must_be_close_to

@@ -132,5 +133,58 @@

describe "total_revenue" do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should also have tests for creating a driver with invalid arguments.

expect(passenger.net_expenditures).must_be_instance_of Float
end

it "returns the correct net expenditures and total time spent if the passenger's trip is still in-progress" do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants