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

Tanja Stroble -- Carets Ride Share #23

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

Conversation

misstonbon
Copy link

No description provided.

Copy link
Collaborator

@CheezItMan CheezItMan left a comment

Choose a reason for hiding this comment

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

A few comments, a creative approach to organizing the data, but a little difficult to maintain.

i = 0
drivers = [ # drivers array
#index 0 of drivers array
driver1 = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need things like this driver1 = { here.

I also think making these internal hashes containing 3 arrays, rather awkward to traverse. It's also problematic if you accidentally add an element to one array and not the others, they become out of synch.

I would suggest instead doing something like:

driver = [
  {RD0073: [
    {ride_id: "RD0003", rating: 3, pay: 10, dates_active: "2016-02-03"},
    {ride_id: "RD0004", rating: 4, pay: 12, dates_active: "2016-02-07"}
    ]
  }
]    

That makes it easier to loop through things and keeps trips together logically.

drivers.each do |driver|
i += 1
puts "Driver#{i} pay: $#{driver[:pay].sum} for #{driver[:ride_id].length} rides." #calculating pay and ride number
puts "Driver#{i}'s average rating is #{(driver[:rating].inject(0.0) { |sum, el| sum + el } / driver[:rating].size).round(2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice work using an enumerable (inject) to sum up the pay.

Also good work using round.

@CheezItMan
Copy link
Collaborator

Feedback provided in code notes

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