Skip to content

Branches - Erika#40

Open
emaust wants to merge 1 commit intoAda-C12:masterfrom
emaust:master
Open

Branches - Erika#40
emaust wants to merge 1 commit intoAda-C12:masterfrom
emaust:master

Conversation

@emaust
Copy link

@emaust emaust commented Aug 12, 2019

ride share

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
What did your data structure look like at first? Did this structure evolve over time? Why?
What was your strategy for going through the data structure and gathering information?
My strategy was to break down each "block" of information so that each layer could be accessed by a key
What was an example of something that was necessary to store in a variable? Why was it necessary, useful, or helpful?
What kinds of iteration did you use? Did you use .map? If so, when? If not, why, or when would be a good opportunity to use it?
Were some calculations easier than others? Why?

@jmaddox19
Copy link

jmaddox19 commented Aug 15, 2019

Ride Share

What We're Looking For

Feature Feedback
Answers the comprehension questions Please answer the questions next time. They're helpful in reflecting on the assignment and learning from it.
Readable code with consistent indentation and reasonable code style X
Outputs the correct number of rides each driver has given X
Outputs the total amount of money each driver has made X
Outputs the average rating for each driver X
Outputs which driver made the most money X
Outputs which driver has the highest average rating X

Love the way you organized the data by driver, by ride!
Good use of built-in methods like sum, max, map, etc.!
Just a few learning items I pointed out in comments.

So sorry for the previous review!

end


prof_one = profit_per_driver(drive_history,:DR0001).sum

Choose a reason for hiding this comment

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

Considering this is called profit_per_driver, it'd be preferable that it found and returned the sum of the profit rather than an array that still needs to be summed.

max = profit_array.max

puts " *** Max Profit:"
if max == prof_one

Choose a reason for hiding this comment

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

This is not critical but I wanted to mention: this is a good case where you could keep up with the driver name for each profit so that you could read the driver_id rather than creating a whole conditional chain like this.
One way to do that would be:

prof_one = { driver_id: :DR0001,  profit: profit_per_driver(drive_history,:DR0001).sum }
...
max = profit_array.max_by | profit_hash | 
  profit_hash.profit
end
puts "The maximum was earned by driver #{max.driver_id} with a total profit of $#{max.profit}."

The reason I want to call this out is because if we had 100 drivers, the code below would be unmanageable. Reach out to me if that doesn't make sense.

averages_array = [average_one, average_two, average_three, average_four]
max_average = averages_array.max

puts " *** Highest Driver Rating: "

Choose a reason for hiding this comment

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

Same down here about keeping up with the driver name so that you don't need a whole conditional chain.

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.

2 participants