I have a table that holds a user's weight for specific days. In my weights_controller.rb index action I can show these weights - no problem. What I also want to show on the index is the user's current weight, his weight loss over time as well as the timeframe he achieved this weight loss.Thus I created a weight_calculator.rb to do the needed calculations:class WeightCalculator  def initialize(profile, user)     @profile = profile     @user = user  end  def current_weight    @current_weight = Weight.where(:user_id => @user.id).order(:day).last  end  def weight_loss    @current_weight = Weight.where(:user_id => @user.id).order(:day).last    @start_weight = Weight.where(:user_id => @user.id).order(:day).first    @weight_loss = @current_weight.kilograms - @start_weight.kilograms  end  def timeframe    @current_weight = Weight.where(:user_id => @user.id).order(:day).last    @start_weight = Weight.where(:user_id => @user.id).order(:day).first    @weight_loss_timeframe = @current_weight.day - @start_weight.day    @weeks = @weight_loss_timeframe.to_i/7  endendThis is my weights_controller.rb index action:  def index    @weights = Weight.where(:user_id => current_user.id).order(day: :desc)    @weight_loss = WeightCalculator.new(@profile, current_user).weight_loss    @weeks = WeightCalculator.new(@profile, current_user).timeframe    @result = {}    Weight.where(:user_id => current_user.id).each do |weight|      @result[weight.day] = weight.kilograms    end    @profile = Profile.where(:user_id => current_user.id)  endEverything works fine in this setup. However, there is clearly some repition in my weight_calculator.rb and I am asking myself what would be the best way to write this code to be DRY conform?
Weight-loss calculator
ruby;ruby on rails;calculator
There is really quite a lot wrong here.As a very simple improvement, why aren't you using ActiveRecord's associations? Why do you constantly do this...Weight.where(:user_id => current_user.id).each ...# ...@current_weight = Weight.where(:user_id => @user.id).order(:day).lastInstead of this?current_user.weights.each ...# ... current_weight = @user.weights.order(:day).lastThat alone would drastically improve the readability of your code.You can also make the loop in your index action more idiomatic:@result = current_user.weights.map do  [weight.day, weight.kilograms]end.to_hYou also do some weird things in WeightCalculator with instance vairables. Your methods all seem to both return a value, and assign that same value to an instance vairable that is never used. There is no reason to do this:def current_weight  # ...  @current_weight = <calculation>endInstead, you should just return the calculation, or memoize it via ||=:def current_weight  # ...  @current_weight ||= <calculation>endIf I were working on this code, I would refactor it in the following way to take advantage of the above few points:class WeightCalculator  def initialize(profile, user)     @profile = profile     @user = user  end  def weights    Weight.where(:user_id => @user.id)    # or this, doesn't matter:    @weights ||= @user.weights  end  def start_weight    @start_weight ||= weights.order(:day).first  end  def current_weight    @current_weight ||= weights.order(:day).last  end  def weight_loss    current_weight.kilograms - start_weight.kilograms  end  def timeframe    (current_weight.day - start_weight.day).to_i / 7  endendYour index action is also pretty broken.First, You try to access @profile to pass it to your WeightCalculator before you set it, and then later you set it to Profile.where(...), which returns an association, not a specific profile. Again, using associations, you probably just want@profile = current_user.profileSecond, you query Weights all over the place, over and over. You constantly go through the effort of typing out Weight.where(...) instead of just doing it once and reusing the returned set of weights.  You even make multiple calculators for absolutely no reason, and inside the calculator you don't even reuse weights! Just do things once, there is no reason to continually run the same queries over and over, even if they are being cached by ActiveRecord.calculator = WeightCalculator.new(@profile, current_user)@weights = calculator.weights@result = @weights.map do |weight|  [weight.day, weight.kilograms]end.to_h