0

The following code gives me NameError: undefined local variable or method `dir' in extract_snapshots method.

The code is intended to extract snapshots from a video, store them in a created temporary directory, send the snapshots to a service and remove the directory after.

  def perform
    using_temporary_directory do |dir|
      extract_snapshots
      send_snapshots
    end
  end

  def using_temporary_directory(&b)
    Dir.mktmpdir { |dir| b.call(dir) }
  end

  def extract_snapshots
    system "ffmpeg -i #{video_file_path} -vf fps=1/#{INTERVAL} #{dir}/%04d.jpg"
  end

I thought, that dir variable should be visible in extract_snapshots and send_snapshots, because it is on the same level. But it isn't in the scope of those methods. Is it possible to make dir variable visible without doing the following:

  def perform
    using_temporary_directory do |dir|
      extract_snapshots(dir)
      send_snapshots(dir)
    end
  end

?

mSourire
  • 29
  • 4
  • 1
    Why "make dir variable visible without doing the following"??? Your final example is absolutely the correct way to do it. – Max Sep 03 '19 at 20:33
  • I hoped, to make the code cleaner. – mSourire Sep 03 '19 at 22:35
  • Less code is not always cleaner code. What you're asking for is how to create an _implicit_ dependency between your methods. That is one of the dirtiest anti-patterns there is. – Max Sep 04 '19 at 12:11

1 Answers1

0

You could do something like this, using attr_accessor: 

  attr_accessor :dir

  def perform
    using_temporary_directory do
      extract_snapshots
      send_snapshots
    end
  end

  def using_temporary_directory(&b)
    Dir.mktmpdir do |dir| 
      self.dir = dir
      b.call
    end
  ensure # clear the variable when you're finished
    self.dir = nil
  end

  def extract_snapshots
    system "ffmpeg -i #{video_file_path} -vf fps=1/#{INTERVAL} #{dir}/%04d.jpg"
  end

For an explanation, attr_accessor creates reader and writer methods for you, the equivalent of the following:

def dir
  @dir
end

def dir=(val)
  @dir=(val)
end

N.B. in my opinion there's nothing wrong with doing it the way you've specified towards the end of your question, so pick and choose as suits your circumstances :)

Hope this helps - let me know how you get on or if you have any questions.

SRack
  • 11,495
  • 5
  • 47
  • 60
  • More complicated, more error prone, less flexible. This answers the question as asked but is not a good idea in general. – Max Sep 03 '19 at 20:34
  • Thank you very much. I had an idea with an instance variable, but intended to figure out, whether I miss something in understanding of block's local variable scope. In my example *extract_snapshots* is called in the same context with *dir* variable: `using_temporary_directory { |dir| extract_snapshots }` So, I thought, the variable should be visible inside the method. But it looks like I understand this correctly: the closure would work, if *dir* variable were visible in the context, where *extract_snapshots* is defined. – mSourire Sep 03 '19 at 22:25
  • @Max thanks for the feedback, I'd tend to agree - earlier versions of my question were clearer on this, though I trimmed it down for brevity. Hope this has helped clarify the issue somewhat though mSourire :) – SRack Sep 04 '19 at 08:25