-
Notifications
You must be signed in to change notification settings - Fork 48
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
Space - Diana #30
base: master
Are you sure you want to change the base?
Space - Diana #30
Conversation
Ride ShareMajor Learning Goals/Code Review
Functional Requirements
Overall Feedback
Code Style Bonus AwardsWas the code particularly impressive in code style for any of these reasons (or more...?)
|
date_of_ride: ["3rd Feb 2016", "3rd Feb 2016", "5th Feb 2016"], | ||
rating: [3, 4, 2], | ||
money_earned: [10, 30, 45], | ||
rider_of_drive: ["RD0003", "RD0015", "RD0003"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are going to align these keys and values like this, indent these lines. It makes it more clear what is and isn't in each hash in rides
# (1) Number of rides each driver has given | ||
# Based off of the "date_of_rides" array, finding the | ||
# length of this array will give the total amount of rides | ||
|
||
# Strategy - first try to find amount for one driver first, | ||
# then iterate. | ||
# Came up with the following code for a single driver | ||
## puts "Driver 1 gave #{(rides[:DR0001][:date_of_ride]).length} rides" | ||
|
||
# To iterate the above code to give all drivers amount of rides | ||
# use the ".map" enumerable | ||
# to access the correct array, must include both key (driver) and value (data) | ||
# as arguments for the block |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is pretty big for a standard comment! Can you sum it up in 1-2 lines instead? Comments work best as an in-line tool, snug with the code it's giving info on.
rides.map {|driver, data| | ||
puts "Driver #{driver} made a total of: $#{(data[:money_earned]).inject(:+)}" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is SUPER elegant!
# When iterating, wanted a nonrepeating decimal and used sprintf("%.2f", <average rating>) | ||
# to output only 2 decimal places | ||
|
||
rides.map {|driver, data| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really want map
here? What is map
doing that each
can't?
highest_earnings = 0.0 | ||
highest_earner = " " | ||
|
||
rides.each { |driver, data| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You iterate over this hash a lot. Is there a way to keep this code organized while reducing the number of times you loop over rides
? Perhaps methods could help.
Assignment Submission: Ride Share
Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.
Reflection
.map
? If so, when? If not, why, or when would be a good opportunity to use it?