Melissa_Pushpa_Edges_OO_RideShare #24
Conversation
…cessfully passed.
…ip method as well in driver.rb file, and got tests to pass along with that file.
Ride ShareWhat We're Looking For
This is a good start. The code that I see so far on this looks solid, particularly product code, and particularly what you've written for wave 1. I also want to acknowledge that this was a difficult project and a rough week in general. All that being said, I feel there is a lot of room for improvement in this submission. In my mind, this assignment has 4 core learning goals:
I would say that each of these was touched upon, but none of them addressed completely:
The key thing at this point is to figure out how to make sure these learning goals are met well enough that it won't be an issue once we start Rails. This means the two major points are building complex logic and reading other peoples' code. You'll have plenty of opportunity for complex logic on the Hotel project. One thing I would recommend is to check in 1-on-1 with an instructor tomorrow (Wednesday), to make sure you're on a good track and making solid progress. For reading code and understanding a complex codebase, there is an implementation of GroceryStore on the Again, I want to call out that this project was particularly difficult, and that the code I do see for the most part looks good. Keep your focus and your spirits up, and keep up the hard work! |
| def net_expenditures | ||
| ride_total = 0 | ||
| @trips.each do |trip| | ||
| ride_total += trip[:cost] |
There was a problem hiding this comment.
Both these methods will hit an error if you try to run them on a user with an incomplete trip. There are a couple of workarounds for this.
You could explicitly ignore trips with a nil cost:
trips.each do |trip|
if trip.cost.nil?
next
end
# ... add to the total ...
endOr even better, you could write a helper method that returns a list of complete trips:
def completed_trips
return @trips.reject { |t| t.end_time.nil? }
end
def net_expenditures
completed_trips.each do |trip|
# ... same logic as before ...
end
endThere was a problem hiding this comment.
While the above is true, it is less relevant given that you didn't complete wave 3.
| # create bunny as an instance of a trip. | ||
|
|
||
| bunny = RideShare::Trip.new(trip_data) | ||
| # calls the method duration |
| describe "total amount of money user has spent on trips" do | ||
| before do | ||
| @user = RideShare::User.new(id: 9, name: "Merl Glover III", phone: "1-602-620-2330 x3723", | ||
| trips: [{ |
There was a problem hiding this comment.
I think you're missing some edge cases that are common to both these User methods:
- What happens if the user has no trips?
- What happens if the user has an incomplete trip?
|
|
||
| if input[:id].nil? || input[:id] <= 0 | ||
| raise ArgumentError, 'ID cannot be blank or less than zero.' | ||
| end |
There was a problem hiding this comment.
Doesn't User already check this? If not it probably should.
| # binding.pry | ||
| # unless @status.include?(status_array) | ||
| # raise ArgumentError. "Invalid status, you entered: #{status}" | ||
| # end |
There was a problem hiding this comment.
Why is this commented out? It does look like line 35 is backward (should be unless status_array.include?(@status).
| sum_tax = 1.65 | ||
| total_rev = 0 | ||
| @driven_trips.each do |trip| | ||
| sum = (trip.cost - sum_tax) * 0.8 |
There was a problem hiding this comment.
This code would be a little more readable if the two magic numbers (0.8 and 1.65) were stored in constants, maybe something like DRIVER_CUT and FEE.
|
|
||
| def net_expenditures | ||
| end | ||
|
|
There was a problem hiding this comment.
One possible implementation:
def net_expenditures
return (super - total_revenue).round(2)
endThe key here is taking advantage of code you've already written, both from the parent class using super, and another method in this class.
OO Ride Share
Congratulations! You're submitting your assignment!
Comprehension Questions
UserandDriver