I am assigning values to my class' instance variable, My scenario is that I have to call function atleast 3 times and each call requires the answer to be store in separate instance variable. Keeping in mind the rubocop errors.

My class

class Major

  attr_accessor :max_temp, :min_temp, :max_humid, :max_t_day, :min_temp_day, :max_humid_day

  def initialize
    @max_temp = -440
    @min_temp = 1000
    @max_humid = -500
    @max_t_day = 'fahad'
    @min_temp_day = 'fahad'
    @max_humid_day = 'fahad'
  end

  def day_wise_results
    _arg, year, path, _month = ARGV
    arr = Dir.entries(path).select { |x| x.include?(year) }
    # max_temp_day, min_temp_day, max_humid_day = ''

    arr.each do |yc|
      collection = file_collection("#{path}/#{yc}")
      collection.shift

      temperature_with_day(collection, 1, true, '>')
      temperature_with_day_min(collection, 3, false, '<')
      temperature_with_day_humid(collection, 7, true, '>')
end

and the functions that have exact same code just the instance variable are different and I dont want to repeat code

Functions

  def temperature_with_day(collection, col, is_max, operator)
    if separete_collections(collection, col, is_max).public_send(
      operator, @max_temp
    )
      @max_temp = separete_collections(collection, col,
                                       is_max)
    end
    collection.each do |row|
      @max_t_day = row[0] if row[col].to_i.eql?(@max_temp)
    end
  end

  def temperature_with_day_min(collection, col, is_max, operator)
    if separete_collections(collection, col, is_max).public_send(
      operator, @min_temp
    )
      @min_temp = separete_collections(collection, col,
                                       is_max)
    end
    collection.each do |row|
      @min_temp_day = row[0] if row[col].to_i.eql?(@min_temp)
    end
  end

  def temperature_with_day_humid(collection, col, is_max, operator)
    if separete_collections(collection, col, is_max).public_send(
      operator, @max_humid
    )
      @max_humid = separete_collections(collection, col,
                                        is_max)
    end
    collection.each do |row|
      @max_humid_day = row[0] if row[col].to_i.eql?(@max_humid)
    end
  end

As you can see everything is same inside these three functions, Is there anyway where I don't have to repeat code and I can go with single instance variable and avoid rubocop errors.

also mentioning my final output is all these three instance variables having some value

CodePudding user response：

Could you use a hash instead, passing a symbol to indicate which value you want to use? Something like this, which adds the extra value as the first argument for the method, here called type.

  def initialize
    @temp = { max: -440, min: 1000, humid: -500 }
    @temp_day = { max: 'fahad', min: 'fahad', humid: 'fahad' }
  end

...

  def temperature_with_day(type, collection, col, is_max, operator)
    if separete_collections(collection, col, is_max).public_send(
      operator, @temp[type]
    )
      @temp[type] = separete_collections(collection, col, is_max)
    end
    collection.each do |row|
      @temp_day = row[0] if row[col].to_i.eql?(@temp[type])
    end
  end

If you still need separate methods to set and return the three values, you can add these explicitly instead of relying on the automatic accessors, something like this:

def max_temp=(value)
  @temp[:max] = value
end

def max_t_day
  return @temp_day[:max]
end

and so on.

(PS Your method separete_collections should probably be called separate_collections, I think - just a spelling mistake in "separate"!)

CodePudding user response：

You can use some constant

SETTINGS = {
  max_temp: {
    col: 1,
    is_max: true,
    operator: :>
  },
  # ...
}

And then iterate over its

SETTINGS.each do |temp, opts|
  separete_collections = separete_collections(collection, opts[:col], opts[:is_max])

  if separete_collections.public_send(opts[:operator], public_send(temp))
    public_send("#{temp}=", separete_collections)
  end
end

if (row = collection.find { |row| row[col].to_i == @max_humid })
  public_send("#{temp}=", row[0])
end