1

I have an object Foo. Foo has a controller, and the controller has a standard index that displays all Foos in a table containing each Foo's attributes and allows users to filter and sort Foos by those attributes. This part works just dandy.

class FoosController < ApplicationController
    def index
        @foos = Foo.filter_data_by_params
        @foos = Foo.sort_some_data_by_params

        respond_to do |format|
            format.html { standard_index } # Has link to format xls
            format.xls do
                require_relative '../path/to/spreadsheet_view.rb' # Uh oh.
                create_spreadsheet # method in spreadsheet_view that generates and returns xls file
            end
        end
    end
end

But there's another part of this that seems dangerous to me. The index view also provides users the option to view the gathered index data as a spreadsheet. However, because trying to use XLS ERB returns an XML 2004 file rather than a 97-2004 file compatible with our company's workflow, I have to invoke a regular .rb file that uses the Spreadsheet gem to generate the 97-2004 compatible spreadsheet rather than a .erb file.

def create_spreadsheet
    book = Spreadsheet::Workbook.new
    sheet = book.create_worksheet(:name => 'Sheet1')

    fields = Foo.columns

    sheet.row(0).concat(fields)

    @foos.each_with_index do |foo, index|
        fields.each do |field|
            sheet.row(index + 1).push foo.field
        end
    end

    spreadsheet = StringIO.new
    book.write spreadsheet
    send_data spreadsheet.string, :filename => "foos.xls", :type => "application/vnd.ms-excel"
end

Now, I'm not sure if using an external .rb file rather than an .erb file is necessarily so bad in and of itself, but I do realize that require_relative is probably a terrible way to go about doing it.

Correct me if I'm wrong, but from what I understand, require_relative can only be used once per Rails session (as with any Ruby require) and would define the "create spreadsheet" method on the scope of the entire application rather than for just the current instance of the FoosController I'm working with.

tl;dr: What I need is a way to expose a method to FoosController#index that can use the @foos variable populated by index to generate a spreadsheet, much like a view file would, without screwing everything else up - i.e. it can be edited without having to reload the server and won't pollute the rest of my app.

Vardarac
  • 553
  • 2
  • 16
  • What do you mean by "pollute the rest of my app" - does it change anything in existing classes? – Uri Agassi Nov 17 '14 at 16:34
  • I'm only assuming here... But if I `require` that `.rb` file with a naked method like `create_spreadsheet` above, wouldn't it define `create_spreadsheet` permanently for whatever else might want to call a different `create_spreadsheet` method? I guess my question might better be asked as, "What happens when I require_relative with a naked method? What's a way to include a method from another file once for a particular instance of a class?" – Vardarac Nov 17 '14 at 16:39
  • 1
    Put it inside a `class` or a `module`, and there won't be any problem - this is programming... – Uri Agassi Nov 17 '14 at 16:40
  • There is no such thing to "include once" - `require` _loads_ the file, like `Java`'s `ClassLoader`. It becomes part of the loaded code. There is no need to create "naked" methods in a program larger than a simple script... – Uri Agassi Nov 17 '14 at 16:42

1 Answers1

1

Normally this is handled with Concerns in Rails 4. You can achieve a similar effect in Rails 3 by declaring a module and using that:

module SpreadsheetCreator
  def create_spreadsheet
    # ...
  end
end

So long as this is named spreadsheet_creator.rb and is in a path that the ActiveSupport autoloader can find it, you don't need to do anything special to include it. app/helpers or app/models should be one of those spots even if it's less than ideal. Rails 4 has a concerns location for non-model, non-controller helpers like this.

Then in the other context:

class FoosController < ApplicationController
  include SpreadsheetCreator
end

This is pretty sloppy design though, as your creator is really worming in to the controller and making a lot of assumptions about how that controller is structured. For example, there's no clear dependency on @foos, it just magically propagates. Sharing instance variables like that between separate implementation files complicates understanding of your code.

What's better is to make a self-contained helper class with no assumptions, instead well-defined inputs and outputs:

class SpreadsheetCreator
  def initialize(foos)
    @foos = foos
  end

  def string
    book = Spreadsheet::Workbook.new
    sheet = book.create_worksheet(name: 'Sheet1')

    fields = Foo.columns

    sheet.row(0).concat(fields)

    @foos.each_with_index do |foo, index|
      fields.each do |field|
        sheet.row(index + 1).push foo.field
      end
    end

    spreadsheet = StringIO.new
    book.write spreadsheet

    spreadsheet.string
  end
end

Then your re-written controller has a clear hand-off and preserves full control over what's being output from the controller itself:

 respond_to do |format|
   format.html { standard_index } # Has link to format xls
   format.xls do
     spreadsheet = SpreadsheetCreator.new(@foos)

     send_data spreadsheet.string, filename: "foos.xls", type: "application/vnd.ms-excel"
   end
 end

Handing over responsibility to some mystery module is usually a sign you're going against the grain.

Community
  • 1
  • 1
tadman
  • 194,930
  • 21
  • 217
  • 240
  • For this particular use case, I ended up moving the code from the separate rb file into my controller as a private method that was passed the `@foos` instance variable. The method has the `send_data` separated from it like your answer suggests. I miss having a file in views where I feel presentation-related code should go, but perhaps this is the next best thing. – Vardarac Nov 17 '14 at 18:59
  • Always think about how you might test this sort of thing: A test is easier to write if it has fewer dependencies and integration points. Sounds like you're on the right track now. – tadman Nov 17 '14 at 19:02