it blew up

[djbender]

class Library                                     
  SYSTEMS = ['arcade', 'atari', 'pc']             

  attr_accessor :games                            

  def method_missing(name, *args)                 
    system = name.to_s                            
    if SYSTEMS.include?(system)                   
      define_method system do                     
        find_by_system(sytem)                     
      end                                         
      send(system)                                
    end                                           
  end                                             

  private

  def find_by_system(system)
    games.select { |game| game.system == system }
  end
end

Library.new.atari # =>

SeeingIsBelieving::VERSION  "2.1.4"                                                            
Parser::VERSION             "2.1.9"                                                            
RUBY_VERSION                "2.1.5"                                                            
ENV['RUBY_VERSION']         "2.1.5"                                                                            

it appears to not blow up if I remove send(system)

[JoshCheek]

SiB issue

Thanks for the report. The issue is because of a stack overflow. In this version of SiB (2.1.4), it can't tell the difference between a stack overflow in your code, and an error in its code, so it assumes that SOs are its fault and gives that report. I've got a beta version for 3.0 out, where this is resolved (I think from this commit), but it's got a lot of other bugs that need to be fixed. Hopefully this month ^_^

Code issue

With regards to the example code, the issue is that define_method comes from Module, and is thus not available to the instance of Library (you'd call it on the class instead of the instance). This causes it to go back into method missing, with name set to :define_method which fails the if conditional. Since there is no call to super for examples where name is not in SYSTEMS, it then simply returns from method_missing without raising an error to alert you of the problem. If you then call send(system), well system is :atari, so that tries to call the method that was expected to just be defined. Since it wasn't, that goes back into method_missing, attempts to define it again, fails again, but the failure is again caught by a call to method_missing and simply not defined, and then it again gets to the send(system) which again tries to call the method that it again failed to define, which gets caught by method_missing, which .... and eventually the machine runs out of memory.

How to fix it

I'm not totally sure what you're going for, but either of these would work to fix your particular issue.

self.class.__send__ :define_method, system do
  find_by_system(sytem)
end

define_singleton_method system do                     
  find_by_system(sytem)
end

A change that would avoid this kind of issue

Calling super in method_missing would be helpful for you in general, and would avoid this issue in SiB:

def method_missing(name, *args)
  super unless SYSTEMS.include? name  # ~> NoMethodError: undefined method `define_method' for #<Library:0x007fc392896158>
  define_method name do
    find_by_system name
  end
  __send__ name
end                                   # => :method_missing

Alternative to method_missing

An observation: Metaprogramming in production code has never provided me the benefits I expected from it, tends to be harder to understand and maintain, and it usually is full of confusing issues like this one.

Here is an alternative that doesn't use method_missing

class Library
  @systems = []
  def self.add_system(name)
    @systems << name
    define_method name do
      games.select { |game| game.system == name }
    end
  end

  add_system :arcade
  add_system :atari
  add_system :pc

  attr_reader :games
  def initialize(games)
    @games = games
  end
end

Alternative to Metaprogramming

In general, there's rarely ever a point to metaprogramming. In this case, we're passing arguments via method names. if we pass them as regular arguments, the code gets shorter, simpler to understand, easier to think about, and easier for the runtime to optimize:

class Library
  SYSTEMS = [:arcade, :atari, :pc]

  attr_reader :games
  def initialize(games)
    @games = games
  end

  def for(system)
    SYSTEMS.include?(system) || raise(ArgumentError, "System #{system.inspect} not in #{SYSTEMS.inspect}")
    @games.select { |game| game.system == system }
  end
end

Game = Struct.new :name, :system

games = [
  Game.new("arcade1 game 1", :arcade),
  Game.new("atari game 1",   :atari),
  Game.new("pc game 1",      :pc),
  Game.new("arcade1 game 2", :arcade),
  Game.new("atari game 2",   :atari),
  Game.new("pc game 2",      :pc),
]

Library.new(games).for(:atari)
# => [#<struct Game name="atari game 1", system=:atari>,
#     #<struct Game name="atari game 2", system=:atari>]

[djbender]

Wow thorough! This is a coding exercise from code school that I was investigating via SiB.

Cheers!

Sent from my iPhone

On Jan 5, 2015, at 3:17 AM, Josh Cheek notifications@github.com wrote:

SiB issue

Thanks for the report. The issue is because of a stack overflow. In this version of SiB (2.1.4), it can't tell the difference between a stack overflow in your code, and an error in its code, so it assumes that SOs are its fault and gives that report. I've got a beta version for 3.0 out, where this is resolved (I think from this commit), but it's got a lot of other bugs that need to be fixed. Hopefully this month ^_^

Code issue

With regards to the example code, the issue is that define_method comes from Module, and is thus not available to the instance of Library (you'd call it on the class instead of the instance). This causes it to go back into method missing, with name set to :define_method which fails the if conditional. Since there is no call to super for examples where name is not in SYSTEMS, it then simply returns from method_missing without raising an error to alert you of the problem. If you then call send(system), well system is :atari, so that tries to call the method that was expected to just be defined. Since it wasn't, that goes back into method_missing, attempts to define it again, fails again, but the failure is again caught by a call to method_missing and simply not defined, and then it again gets to the sen d(system ) which again tries to call the method that it again failed to define, which gets caught by method_missing, which .... and eventually the machine runs out of memory.

How to fix it

I'm not totally sure what you're going for, but either of these would work to fix your particular issue.

self.class.send :define_method, system do find_by_system(sytem) end define_singleton_method system do
find_by_system(sytem) end A change that would avoid this kind of issue

Calling super in method_missing would be helpful for you in general, and would avoid this issue in SiB:

def method_missing(name, *args) super unless SYSTEMS.include? name # ~> NoMethodError: undefined method `define_method' for #Library:0x007fc392896158 define_method name do find_by_system name end send name end # => :method_missing Alternative to method_missing

An observation: Metaprogramming in production code has never provided me the benefits I expected from it, tends to be harder to understand and maintain, and it usually is full of confusing issues like this one.

Here is an alternative that doesn't use method_missing

class Library @systems = [] def self.add_system(name) @systems << name define_method name do games.select { |game| game.system == name } end end

add_system :arcade add_system :atari add_system :pc

attr_reader :games def initialize(games) @games = games end end Alternative to Metaprogramming

In general, there's rarely ever a point to metaprogramming. In this case, we're passing arguments via method names. if we pass them as regular arguments, the code gets shorter, simpler to understand, easier to think about, and easier for the runtime to optimize:

class Library SYSTEMS = [:arcade, :atari, :pc]

attr_reader :games def initialize(games) @games = games end

def for(system) SYSTEMS.include?(system) || raise(ArgumentError, "System #{system.inspect} not in #{SYSTEMS.inspect}") @games.select { |game| game.system == system } end end

Game = Struct.new :name, :system

games = [ Game.new("arcade1 game 1", :arcade), Game.new("atari game 1", :atari), Game.new("pc game 1", :pc), Game.new("arcade1 game 2", :arcade), Game.new("atari game 2", :atari), Game.new("pc game 2", :pc), ]

Library.new(games).for(:atari)

=> [#<struct Game name="atari game 1", system=:atari>,

<struct Game name="atari game 2", system=:atari>]

— Reply to this email directly or view it on GitHub.