CODEWITHSUNDEEP
rubyruby-on-rails-4ruby-2.1
user2931706
user2931706

Reputation:

Ruby block returning inappropriate value

Following up this screencast, and its someway returning 2..3 instead if Product records

def save
    puts "--- imported_products: #{imported_products.inspect}"
    // --- imported_products: 2..3
    if imported_products.map(&:valid?).all?
      imported_products.each(&:save!)
      true
    else
      imported_products.each_with_index do |product, index|
        product.errors.full_messages.each do |message|
          errors.add :base, "Row #{index+2}: #{message}"
        end
      end
      false
    end
 end

 def imported_products
   @imported_products ||= load_imported_products
 end

 def load_imported_products
   spreadsheet = open_spreadsheet
   header = spreadsheet.row(1)
   (2..spreadsheet.last_row).each do |i|
     row = Hash[[header, spreadsheet.row(i)].transpose]
     product = Product.find_by_id(row['id']) || Product.new
     product.attributes = row.to_hash.slice(*accessible_attributes)
     product
   end
 end

Upvotes: 0

Views: 100

Answers (3)

engineersmnky
engineersmnky

Reputation: 29423

Or use map 

def load_imported_products
  spreadsheet = open_spreadsheet
  header = spreadsheet.row(1)
  products = (2..spreadsheet.last_row).map do |i|
    row = Hash[[header, spreadsheet.row(i)].transpose]
    product = Product.find(row['id'].to_i) || Product.new
    product.attributes = row.to_hash.slice(*accessible_attributes)
    product
  end
end

Also find_by_id is not needed the find method uses id although I forced it to an integer in case it was nil or stored as a string.

Upvotes: 1

user3446496
user3446496

Reputation: 361

If you're talking about the load_imported_products method, then try adding each product to an array, than then return the array.

I'm not sure exactly what that method is returning, but you probably need to explicitly return a collection of products.

So 

def load_imported_products
   products = []
   spreadsheet = open_spreadsheet
   header = spreadsheet.row(1)
   (2..spreadsheet.last_row).each do |i|
     row = Hash[[header, spreadsheet.row(i)].transpose]
     product = Product.find_by_id(row['id']) || Product.new
     product.attributes = row.to_hash.slice(*accessible_attributes)
     products << product
   end
   return products
 end

Upvotes: 0

mattwise
mattwise

Reputation: 1506

Your load_imported_products method includes an each block. That block is the last 'line' of your method, so the return value of the block becomes the return value of the method.

Try the following:

def load_imported_products
  spreadsheet = open_spreadsheet
  header = spreadsheet.row(1)
  products = []
  (2..spreadsheet.last_row).each do |i|
    row = Hash[[header, spreadsheet.row(i)].transpose]
    product = Product.find_by_id(row['id']) || Product.new
    product.attributes = row.to_hash.slice(*accessible_attributes)
    products << product
  end
  products
end

Upvotes: 2

Related Questions