3

I have what I assumed was a very simple method within my controller:

class ReportsController < ApplicationController

  client = MWS.reports

  def request_all_listings
    begin
      parser = client.request_report('_GET_FLAT_FILE_OPEN_LISTINGS_DATA_', opts = {})
      @result = parser.parse["ReportRequestInfo"]["ReportProcessingStatus"]

      puts @result

    rescue Excon::Errors::ServiceUnavailable => e
      logger.warn e.response.message
      retry
    end
  end

  request_all_listings

end

This gives me the error:

 undefined local variable or method `request_all_listings' for ReportsController:Class

What am I doing wrong here? When I delete the def request_all_listings def and end lines and just have the begin/rescue/end my code works fine...

JVG
  • 20,198
  • 47
  • 132
  • 210

2 Answers2

6

request_all_listings is ambiguous. It's either a variable or a class method call. request_all_listings is an object method.

To fix it, you need to either define request_all_listings as a class method.

def self.request_all_listings
    ...
end

Or create an object to call request_all_listings with.

ReportsController.new.request_all_listings

In general it's bad form to do work when a class is loaded. This makes it impossible to load the class without it doing work and can slow things down and make it difficult to use and test.

Instead I'd suggest doing this work when an instance is loaded and caching it in a class instance variable.

class Foo
  # class instance variable
  @all_listings = []

  # class method
  def self.request_all_listings
    puts "Calling request_all_listings"
    @all_listings = [1,2,3]

    return
  end

  # class method
  def self.all_listings
    request_all_listings if @all_listings.size == 0

    return @all_listings
  end

  # object method
  def all_listings
    return self.class.all_listings
  end
end

# request_all_listings is only called once for two objects
puts Foo.new.all_listings.inspect
puts Foo.new.all_listings.inspect
Community
  • 1
  • 1
Schwern
  • 153,029
  • 25
  • 195
  • 336
  • Great, detailed response! I'm very new to Ruby (I'm a Node dev, don't hit me), this is actually for accessing an API so it needs to run every time the associated view is loaded, which seems to be different than what you're suggesting. Is that so? – JVG Feb 03 '16 at 23:36
  • @Jascination No, same thing. I'm guessing this is Rails. Rails (and just about anything else) will only load a class once when the server starts, so with your method `request_all_listings` will only ever run once. Rails (I think) will create an instance of the controller class for each request. In Ruby, everything is about objects. – Schwern Feb 04 '16 at 00:03
0

move the line request_all_listings into the constructor:

def initialize
    request_all_listings
end

When you create a ReportsController instance, initialize will automatically run:

reports = ReportsController.new
Gavriel
  • 18,880
  • 12
  • 68
  • 105
  • Not entirely sure what you mean by this. Why would I need to put `request_all_listings` inside a separate method? If I then need to call `initialize` that just repeats the problem, surely? – JVG Feb 03 '16 at 22:56
  • @Jascination `initialize`, if it exists, is automatically run on object creation. – Schwern Jun 16 '17 at 19:00