Article: Adding Properties to Ruby Metaprogramatically

The subject of this article is good,but the code is full of bugs and style errors.

define_method("#{sym}".to_sym) {
instance_variable_get("@#{sym.to_s}")
}

Why do you convert sym to a string and then back to a symbol? It's more common to use do...end for blocks where the open brace is not on the same line as the end brace.

define_method("#{sym}=".to_sym) {|value|
instance_variable_set("@#{sym.to_s}", value)
}

You don't need to convert the thing to a symbol. define_methods accepts strings too. You don't have to use sym.to_s either as it's already converted to a string by string interpolation. Same thing for do...end.

define_method(:direction=){|value|
instance_variable_set("@#{sym.to_s}", value)
fire_event_for(:direction)
}

Why do you have :direction= and :direction in there, but also #{sym.to_s}? This code will not work.

define_method("add_#{sym}_listener".to_sym){|x|
@listener[:sym] << x
}

to_sym is not necessary here. @listener[:sym] should be @listener[sym].

"The code for setting up the listener lists and the rest is left to the reader as exercise. (Stop moaning, it's just 3 lines of code)."

Could you give these 3 lines? I don't know how to do it (without cheating by putting multiple things on one line). The fire_event_for will be at least three lines of code. Setting up the listener hash table is also hard with this implementation. So again: please show the code :)

h = CruiseShip.new
h.add_direction_listener(Listener.new())
h.add_bar_listener lambda {|x| puts "Oy... someone changed the property to #{x}"}
h.hello = 10

Where do you define the class Listener? Where does add_bar_listener come from? Where does hello= come from? Why does the bar_listener get called if you call hello=(10)?

class Ship
extend Properties
end

Maybe it's good to explain that extend adds the methods to the Ship class object and not to the Ship instances? This is not the common way to use mixins in Ruby. Well maybe it's better to pretend that it's this simple.

property :speed {|v| v >= 0 && vn< 300 }

This code won't parse. You need parens here.

def property(x, &predicate)
define_method("#{sym}=".to_sym) {|arg|
if(!predicate.nil?)
if !predicate.call(arg)
return
end
end
instance_variable_set("@#{sym.to_s}", arg)
prop_holder = instance_variable_get("@#{sym.to_s}_property_holder")
prop_holder.fire_event(arg) if prop_holder
}
end

OK. to_sym not necessary. "!predicate.nil?" is the same as "predicate". Why do you use braces for the first if but not for the second? to_s is redundant (2x). Where do you set @#{sym}_property_holder? What is prop_holder? What is prop_holder.fire_event?

property :speed, in([0..300]

Missing close paren ;-). Why do you put the range in an array?

property :speed, in(0..300)

def in(x)
lambda{|value| x === value}
end

This will allow you to pass a regex:

property :email, in(/email regex/)

But the name "in" isn't very good now. That's not a problem because you cannot use "in" anyway because it's a reserved word.

I hope you fix the errors. This is a great article if you do.

Thank you for pointing out improvements!

Wow, thanks for the detailed comments Jules!
I guess it serves me right for copy/pasting code between evolving source code and an evolving article text - nothing good can come from that.
The more stupid errors will be fixed right away;
Let me address your questions:

• extending a Module
  If I _include_ a Module, the methods will be added as instance methods, but to have
  something like this
  class Foo
  property :xyz
  end
  they need to be available as class methods. With include, you'd call them as instance
  methods so... I guess you'd have to setup the properties in the constructor or somewhere else. If I'm missing a better way to do this, I'd be interested to hear;
  
• the do/end vs braces thing...
  well, I don't know, I like the braces; there's a precedence difference between the two, but otherwise it seems to be a matter of taste... also: the word "do" feels like clutter, and I wanted to make the solution seem as clean as possible; There's no real reason why there's a difference for delimiting blocks - Smalltalk uses brackets for _all_ blocks;
  
• thanks for the === tip;
  
• good catch on the "in" keyword... I updated the article to mention this; however, the ideas in this section were thought of as possible notations, that the user could play with (as always with internal DSLs, it's good to come up with the look, then chip away at it and prop some parts up to make it Ruby code);.
  

Again... thanks for keeping me honest, Jules!

This include vs extend issue made me curious... I looked into Rails (actually ActiveRecord) and how it does it's has_many etc methods, and it also uses extend (grep the Rails source code for "extend" or "base.extend"). Again, if you want the fancy, declarative look, this seems to be the only way.

Here's a nice trick (I think Rails uses a similar thing):

module Example
  def instance_method
    # this method will be added to the class as an instance method
  end

  module ClassMethods
    def class_method
      # this method will be added to the class as a class method
    end
  end

  def self.included(klass)
    klass.extend(ClassMethods)
  end
end

class Test
  include Example  # this will call the Example.included callback
end

If you need only class methods just using extend in the class that needs the Properties seems cleaner.

Thanks to alert reader Jules Jacobs for pointing out areas for improvement.

Here's one way to setup the listeners...

module Properties
  
  def self.extended(base) 
    base.class_eval %q{def fire_event_for(sym, *arg) @listener[sym].each {|l| l.call(arg) } end }
  end
  
  def property(sym, &predicate)
    
    ...
  
    define_method("add_#{sym}_listener") do |x| 
      @listener ||= {}
      @listener[sym] ||= []
      @listener[sym] << x
    end
  end

but I'd love to know how you've gotten the predicate business to work.

property :speed {|v| v >= 0 && vn< 300  }

won't even parse.

Ah, looking through Jules's comments, it looks like there was one other error that hasn't been corrected. So here's one implementation. I love it that with Ruby it takes only a minor method signature to property to support the block vs. lambda switch. (I wonder: is there is a way to get the method to support both with a single signature?)

module Properties
  
  def self.extended(base) 
    base.class_eval %q{def fire_event_for(sym, arg) @listener[sym].each {|l| l.call(arg) } end }
  end
  
  # def property(sym, &predicate)
  def property(sym, predicate=nil)
    define_method(sym) do
      instance_variable_get("@#{sym}")
    end
  
    define_method("#{sym}=") do |arg|
      return if !predicate.call(arg) if predicate
      instance_variable_set("@#{sym}", arg)
      fire_event_for(sym, arg)
    end
  
    define_method("add_#{sym}_listener") do |x| 
      @listener ||= {}
      @listener[sym] ||= []
      @listener[sym] << x
    end
    
    define_method("remove_#{sym}_listener") do |x| 
      @listener[sym].delete_at(x)
    end
  end
  
  def is(test)
    lambda {|val| test === val }
  end
end

class CruiseShip
  extend Properties
  
  property :direction
  # property(:speed) {|v| v >= 0 && v < 300 }
  property :speed, is(0..300)
end

h = CruiseShip.new

h.add_direction_listener(lambda {|x| puts "Oy... someone changed the direction to #{x}"})
h.direction = "north"
h.add_speed_listener(lambda {|x| puts "Oy... someone changed the speed to #{x}"})
h.add_speed_listener(lambda {|x| puts "Yo, dude... someone changed the speed to #{x}"})

h.speed = 200
h.speed = 300
h.speed = 301
h.speed = -1
h.speed = 2000

puts h.direction
puts h.speed

h.remove_speed_listener(1)

h.speed = 200
h.speed = 350

puts h.direction
puts h.speed