Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trang & Daniela - RideShare #5

Open
wants to merge 43 commits into
base: master
Choose a base branch
from
Open

Trang & Daniela - RideShare #5

wants to merge 43 commits into from

Conversation

tfrego
Copy link

@tfrego tfrego commented Aug 31, 2018

OO Ride Share

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe a design decision you had to make when working on this project. What options were you considering? What helped you make your final decision? When establishing the Driver Class, we decided to create a new initialize method instead of inheriting the User super class initialize method. We saw in the test file, the User class took a hash argument while the Driver class took keyword arguments. If the tests had not been pre-written, we most likely would've kept it consistent and used super and a hash.
Describe any examples of composition that you encountered in this project, if any TripDispatcher has drivers, passengers, and trips with a 1 to many relationship. Drivers and Passengers both have trips with a 1 to many relationship. Trips have both passengers and drivers.
Describe the relationship between User and Driver Driver is a subclass of User. It inherits the add_trip and total_time_spent methods. It overrides net_expenditures to account for total_revenue.
Describe a nominal test that you wrote for this assignment. We created many nominal trip cases to calculate the net expenditure for a nominal test. These test cases had trip costs ranging from $5 to $10.
Describe an edge case test that you wrote for this assignment When testing average rating, we tested an edge case where the driver had 0 trips. This was to ensure that our program did not attempt to divide by zero.
Describe a concept that you/your pair gained more clarity on as you worked on this assignment We gained more clarity on the syntax of using classes, creating new instances and referring to them in the tests. We became more comfortable assigning and referencing instance variables. We also gained more clarity using Let. We were able to utilize more enumerables and ternary operators.
What are two discussion points that you and your pair discussed when giving/receiving feedback from each other that you would be willing to share? We felt that we both have the same communication style and that helped us to collaborate. We both got into a groove with our pair programming roles that when one is the driver, the navigator is helping with the correct syntax and methods to invoke, which helped speed up the project.

tfrego and others added 30 commits August 27, 2018 15:20
…r who hasnt driven for the longest time. tests passed.
@dHelmgren
Copy link

Ride Share

What We're Looking For

Feature Feedback
Baseline
Used Git Regularly Yes!
Answer comprehension questions Good examples across the board.
Wave 1
Appropriate use of Ruby's Date Yes!
Trip has a helper method to calculate duration Yes!
User (passenger) has a method to calculate total cost of all trips Indeed!
Tests for wave 1 They look good!
Wave 2
Driver inherits from User Yes!
Driver has add_driven_trip method Check!
Driver has method to calculate average rating Check!
Driver has method to calculate net expenditures and it uses super Check!
Driver has a method to calculate total revenue Check!
Tests for wave 2 Good job covering lots of edge cases!
Wave 3
TripDispatcher has a new method to create trips Yep!
creating a trip in TripDispatcher relies on methods in Driver and User (passenger) to modify their own attributes They do!
Complex logic was correctly implemented Thumbs up!
Tests for request_trip Good tests!
Methods from wave 1 and 2 handle incomplete trips They do!
Tests for wave 1 and 2 methods with incomplete trips Yep!
Wave 4 (Optional)
TripDispatcher now assigns trips to either the newest driver (no trips), or the driver who has not driven in the longest time Good work here!
Appropriate helper methods were made to help with complex logic The way that you wrote this doesn't lend itself to further helpers!
Tests for wave 4 GOOD TESTS!
Overall Nice work across the board. Note that you can spot warnings at the top of the output when you run your rake.

def request_trip(user_id)
available_drivers = @drivers.select {|driver| driver.status == :AVAILABLE and driver.id != user_id }

driver = available_drivers.find {|driver| driver.driven_trips.empty?}

Choose a reason for hiding this comment

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

Check your warnings. It doesn't make a huge amount of difference in THIS code, but here and on line 102, you have a "shadowed variable". driver on the left hand side of this assignment is being covered up by |driver| inside this find, and it creates a situation that is ambiguous to the compiler and ambiguous to a reader

start_time = Time.parse('2015-05-20T12:14:00+00:00')
end_time = start_time + 25 * 60 # 25 minutes
@trip_data = {
data = {

Choose a reason for hiding this comment

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

This creates a compiler warning too. The way to keep the compiler from complaining is to explicitly return data.

@@ -22,5 +26,10 @@ def inspect
"ID=#{id.inspect} " +
"PassengerID=#{passenger&.id.inspect}>"
end

def trip_duration
return @end_time == nil ? nil : duration = @end_time - @start_time

Choose a reason for hiding this comment

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

Nice!


end

class Driver < User

Choose a reason for hiding this comment

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

For future's reference, at least while you're still less experienced with Ruby, I'd avoid declaring 2 classes inside the same file. Especially in Ruby, this can hide problems with your code during the testing process.

def total_revenue
sum = 0

@driven_trips.each do |driven_trip|

Choose a reason for hiding this comment

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

Don't forget that in Ruby you have access to methods like Sum! I don't always think they're the best choice for every situation, but they can be an effective way to concisely communicate your thinking to readers!

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