Undefined method `convert' for FileUpload:Class

Ruby on Rails
1

I have created a FileUpload class and I am now trying to re-factor it a little bit, but I am getting a undefined method error,

below is the code before I started to re-factor:

class FileUpload
  def self.import(file, user)
    CSV.foreach(file.path, headers: true) do |row|
      company = Company.find_by(name: row[0])
      row = row.to_hash
      row.merge! user_id: user.id
      row.delete("company")
      company.job_sheets.create! row
    end
  end
end

and this is the code after which I am getting the error, I don’t quite understand what I have done wrong, as the method I have created is there :S

class FileUpload
  def self.import(file, user)
    CSV.foreach(file.path, headers: true) do |row|
      company = Company.find_by(name: row[0])
      converted_data = convert(row)
      company.job_sheets.create! converted_data
    end
  end

  def convert(row)
    row = row.to_hash
    row.merge! user_id: user.id
    row.delete("company")
  end
end

Any help is appreciated.

2

After reading the error again I realised I needed self.convert

below is the code:

  def self.import(file, user)
    CSV.foreach(file.path, headers: true) do |row|
      company = Company.find_by(name: row[0])
      converted_data = convert(row, user)
      company.job_sheets.create! converted_data
    end
  end

  def self.convert(row, user)
    row = row.to_hash
    row.merge! user_id: user.id
    row.delete("company")
    row
  end
end

If anybody can help re-factor any more please do :smile:

3

So one thing that immediately jumps out to me is I don’t like mixing my key types. When your adding the user_id key you use a symbol while the hash already uses string keys. In rails we have HashWithIndifferentAccess which allows us to use symbols to access keys that are strings. It wouldn’t be obvious outside of this method that the hash returned from covert has all string keys except for user_id.

Apart from that you could certainly remove the converted_data variable… that’s up to personal taste though, I like the explicitly named variable but not sure how much information it truly adds.

def self.import(file, user)
    CSV.foreach(file.path, headers: true) do |row|
      company = Company.find_by(name: row[0])
      company.job_sheets.create! convert(row, user)
    end
  end

  def self.convert(row, user)
    row = row.to_hash
    row.merge! { 'user_id' => user.id }
    row.delete('company')
    row
  end
end