patternrubyrailsMinor
Math Calculus in ruby
Viewed 0 times
mathrubycalculus
Problem
I have several calculus to do in my ruby app.
My code is currently working but I find it very ugly.
Any idea how to make it still readable but less ugly ?
My code is currently working but I find it very ugly.
@guestreviews = GuestReview.where(:reviewed_id => @user.id)
@hostreviews = HostReview.where(:reviewed_id => @user.id)
@hospitalityrate = 0
@foodrate = 0
@placerate = 0
@hostreviews.try(:each) do | rate |
@hospitalityrate += rate.people_rate
@foodrate += rate.food_rate
@placerate += rate.place_rate
end
@hospitalityrate = (@hospitalityrate / @hostreviews.size).round
@foodrate = (@foodrate / @hostreviews.size).round
@placerate = (@placerate / @hostreviews.size).round
@averagerate = ((@hospitalityrate + @foodrate + @placerate) / @hostreviews.size).roundAny idea how to make it still readable but less ugly ?
Solution
It's actually easier to do this in the database layer. It's exceedingly good at these kinds of things.
It'll result in more database queries, but overall it's more efficient.
I've left out the
You could also add a
If you do that, you could also consider simply moving the calculations to the
Lastly, (and this is just small stuff) I'd probably rename some of the variables. I.e. call it
Similarly, I'd call the first average
(Edit: Actually, it's more natural to prefix the
In the code above, I've also underscored the names, just to keep things consistent with general Rails/Ruby conventions (
Edit: Just for fun, you could go fully meta (though it's probably more confusing than anything - the above is much more readable)
That'll set
@guest_reviews = GuestReview.where(:reviewed_id => @user.id)
@host_reviews = HostReview.where(:reviewed_id => @user.id)
@hospitality_rate = @guest_reviews.average('people_rate')
@food_rate = @guest_reviews.average('food_rate')
@place_rate = @guest_reviews.average('place_rate')
@average_rate = [@hospitality_rate, @food_rate, @place_rate].inject(:+).to_f / @host_reviews.countIt'll result in more database queries, but overall it's more efficient.
I've left out the
round() call on the @average_rate only because that seems like a presentation concern to me. Let the controller figure out the average (decimals and all), and let the view decide how to round and display it.You could also add a
has_many :guest_reviews, :foreign_key => "reviewed_id" to the User model, so you can just call @user.guest_reviews.average(...). Same for host_reviewsIf you do that, you could also consider simply moving the calculations to the
User model, so you don't need to do it all in the controller. And perhaps you could cache the results on the user record and only recalculate when reviews are added/deleted/changed. Assuming this happens less often than the averages being displayed, it'll speed up things a bit.Lastly, (and this is just small stuff) I'd probably rename some of the variables. I.e. call it
@food_rate_average instead of @food_rate in order to a) indicate that it's an average, and b) so it's not confused with the database column of the same name.Similarly, I'd call the first average
people_rate_average rather than hospitality_rate; the column is called people_rate so it's probably best to keep it consistent with that name (or rename the database column, if "hospitality" is a more fitting name)(Edit: Actually, it's more natural to prefix the
average_ rather than suffixing it, i.e. average_food_rate, average_rate, etc.)In the code above, I've also underscored the names, just to keep things consistent with general Rails/Ruby conventions (
FooBar → foo_bar).Edit: Just for fun, you could go fully meta (though it's probably more confusing than anything - the above is much more readable)
@guest_reviews = GuestReview.where(:reviewed_id => @user.id)
@host_reviews = HostReview.where(:reviewed_id => @user.id)
@average_rate = %w(people_rate food_rate place_rate).map { |column|
instance_variable_set(:"@average_#{column}", @guest_reviews.average(column))
}.inject(:+).to_f / @host_reviews.countThat'll set
@average_people_rate, @average_food_rate and so onCode Snippets
@guest_reviews = GuestReview.where(:reviewed_id => @user.id)
@host_reviews = HostReview.where(:reviewed_id => @user.id)
@hospitality_rate = @guest_reviews.average('people_rate')
@food_rate = @guest_reviews.average('food_rate')
@place_rate = @guest_reviews.average('place_rate')
@average_rate = [@hospitality_rate, @food_rate, @place_rate].inject(:+).to_f / @host_reviews.count@guest_reviews = GuestReview.where(:reviewed_id => @user.id)
@host_reviews = HostReview.where(:reviewed_id => @user.id)
@average_rate = %w(people_rate food_rate place_rate).map { |column|
instance_variable_set(:"@average_#{column}", @guest_reviews.average(column))
}.inject(:+).to_f / @host_reviews.countContext
StackExchange Code Review Q#25172, answer score: 5
Revisions (0)
No revisions yet.