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

Sammi Jo & Jazz 00 Ride Share #19

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

Conversation

jazziesf
Copy link

@jazziesf jazziesf commented Sep 1, 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? Sammi Jo had a great idea following our first day of work to draw an outline of all of the files to see how each file related rather than switching between all of the files. A design decision we made was to store Trips#@driver and Trips#@Passenger as integers (ID numbers) rather than the entire object associated with that number. We ultimately chose that because it seemed like a lot of potential errors could happen with those circular dependencies built in to the code. We had to refactor our tests and code in order to accommodate that change, but we felt it was worth it.
Describe any examples of composition that you encountered in this project, if any Like we said above, we removed the "has-a" relationship between Trips and User and between Trips and Driver.
Describe the relationship between User and Driver Driver inherits from User.
Describe a nominal test that you wrote for this assignment. Does it load driver, does it calculate ratings, and driver and rider cost.
Describe an edge case test that you wrote for this assignment To account for nil if trips are in progress, driver and passenger information doesn't exist, and that wave 1 and 2 code ignores in progress trips.
Describe a concept that you/your pair gained more clarity on as you worked on this assignment calling methods. Before this project, SJ was having a tough time understanding how inheritance and super work in practice. SJ helped me have a better understanding of how to call methods when they are nested. Still a work in progress but have a better understanding.
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 did a good job molding with each others' styles and putting problem-solving on hold when we needed to take time just getting a better conceptual grasp on things. We're both guess people, so that made things easy. It was an even flow of challenges and successes.

@CheezItMan
Copy link

Ride Share

What We're Looking For

Feature Feedback
Baseline
Used Git Regularly Good number of commits, mostly good commit messages, Do try to avoid using waves to describe the commits, instead describe the functionality implemented.
Answer comprehension questions Check, interesting choice about trips and driver & passenger. I'm glad you seem to have worked well together.
Wave 1
Appropriate use of Ruby's Time Check
Trip has a helper method to calculate duration Check, but it wasn't adapted to handle incomplete trips
User (passenger) has a method to calculate total cost of all trips Check
Tests for wave 1 Check
Wave 2
Driver inherits from User Check
Driver has add_driven_trip method Check
Driver has method to calculate average rating Check, but it's not handling incomplete trips
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 Check
Wave 3
TripDispatcher has a new method to create trips Check
creating a trip in TripDispatcher relies on methods in Driver and User (passenger) to modify their own attributes Check
Complex logic was correctly implemented Check, mostly, see my inline notes
Tests for request_trip Check, well done
Methods from wave 1 and 2 handle incomplete trips Average rating wasn't updated.
Tests for wave 1 and 2 methods with incomplete trips Some of the methods are tested, not all
Overall Overall nice work, a few things missed, but you achieved the learning goals of the project.

return 0
else
sum = 0
@driven_trips.each do |trip|

Choose a reason for hiding this comment

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

This would need to be adapted to incomplete trips.

complete_trips = @driven_trips.select { |trip| trip.rating != nil }
sum = complete_trips.sum_by { |trip| trip.rating }
return sum.to_f / complete_trips.length

def total_revenue
sum = 0
@driven_trips.each do |trip|
if trip.cost == nil

Choose a reason for hiding this comment

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

👍

return revenue = (sum * 0.80)
end

def net_expenditures # driver.net_expenditures

Choose a reason for hiding this comment

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

👍

available = @drivers.find { |driver|
driver.status == :AVAILABLE && driver.id != user_id
}
if available == nil

Choose a reason for hiding this comment

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

available can't be nil instead you should check to see if available.empty?

if delay < DELAY_LIMIT
puts "#{exception.message} Please wait. (current delay @ #{delay}" \
" of #{DELAY_LIMIT})"
sleep(5)

Choose a reason for hiding this comment

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

why sleep?

driver = RideShare::Driver.new(id: 54, name: "Rogers Bartell IV",
vin: "1C9EVBRM0YBC564DZ")
vehicle_id: "1C9EVBRM0YBC564DZ")
expect(driver.average_rating).must_equal 0
end

it "correctly calculates the average rating" do

Choose a reason for hiding this comment

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

You also need a test here for handling the average with some incomplete trips.

expect((dispatcher.find_driver(5).driven_trips).length).must_equal (old_number_of_driven_trips + 1)
end

it "correctly handles NO AVILABLE DRIVERS situation" do

Choose a reason for hiding this comment

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

🥇

@@ -59,4 +58,42 @@
end
end
end

describe "net expenditures" do

Choose a reason for hiding this comment

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

You should also test when there are no trips for the user.

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