Skip to content

branches- vi rideshare#47

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

branches- vi rideshare#47
criacuervos wants to merge 1 commit intoAda-C12:masterfrom
criacuervos:master

Conversation

@criacuervos
Copy link

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? My data structure was at first different hashes with all the information in each trip, without any sort of hierarchy. Then I read through the questions and saw many were about the drivers, so I wanted to nest some data in each driver.
What was your strategy for going through the data structure and gathering information? I thought it out and realized I wanted it to be nested in a larger set of data.
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?

@tildeee
Copy link

tildeee commented Aug 19, 2019

Ride Share

What We're Looking For

Feature Feedback
Answers the comprehension questions Some answers were missing; please spend some time on future questions :D
Readable code with consistent indentation and reasonable code style yes! Fantastic work
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

Hi Vi! I am REALLY excited and impressed and happy with this project submission! The code is extremely clear, readable, concise, clever, logical, and accurate. Well done!

I have a few comments on code style, but overall, I see some great code here. Well done!

@@ -0,0 +1,104 @@
########################################################
Copy link

Choose a reason for hiding this comment

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

Now that we're working in git, we'll have better ways to upload files, but I wanna point out that you named this file after your PR, which is nice, but I wanted this as worksheet.rb instead of branches- vi rideshare :)


#Loop inside the drivers hash
#List each key along with the # of elements in its associated array.
drivers.each do |driverName, rides|
Copy link

Choose a reason for hiding this comment

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

These variable names prove to me that you know the structure of your data deeply. Nice work!


#Loop inside the drivers hash
#List each key along with the # of elements in its associated array.
drivers.each do |driverName, rides|
Copy link

Choose a reason for hiding this comment

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

In Ruby, we tend to name variables using snake_case, meaning instead of driverName I may see driver_name instead


#Loop inside drivers hash
drivers.each do |driverName, trips|
total = trips.inject(0) { |sum, trip| sum + trip[:cost].to_i}
Copy link

Choose a reason for hiding this comment

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

Nice use of inject!

#3. The average rating for each driver

drivers.each do |driverName, trips|
avg_rating = trips.inject(0) { |sum, trip| sum + trip[:rating].to_i } / trips.size
Copy link

Choose a reason for hiding this comment

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

It would probably be a more accurate calculation if we did trip[:rating].to_f instead of to_i

rating_winner_driver = ""

drivers.each do |driverName, trips|
avg_rating = trips.inject(0) { |sum, trip| sum + trip[:rating].to_i } / trips.size
Copy link

Choose a reason for hiding this comment

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

You end up having this line that calculates average rating based off of an array called trips duplicated. If you had more time to refactor, I may suggest pulling it into a new method. Imagine a method like this:

def calculate_avg_rating(trips)
  return trips.inject(0) { |sum, trip| sum + trip[:rating].to_i } / trips.size 
end

Therefore, this line may be changed to this:

avg_rating = calculate_avg_rating(trips)

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