Friday, November 12, 2010

Inheritance and class variable

Yesterday I had a great lunch & learn presented by Brian Guthrie titled "Advanced Ruby Idioms So Clean You Can Eat Off Of Them"
It was a great talk and we all learned something interesting about ruby. After the talk Brian and I had a quick discussion about one of the design problems solved in the talk. It is about the inheritance problem in the custom validation framework. That is the subclass (Captain) didn't inherit the validators from the super class(Pirate).
Here is the code


Model classes
class Pirate < BrianRecord::Base
validates_presence_of :parrot
attr_reader :parrot
def initialize(parrot=nil)
@parrot = parrot
end
end

class Captain < Pirate
validates_presence_of :peg_leg
attr_reader :peg_leg
def initialize(parrot, peg_leg)
@parrot, @peg_leg = parrot, peg_leg
end
end




Framework base class
module BrianRecord
class Base

class << self
def validators
@validators ||= []
end

def validates_presence_of(attribute, opts={})
validators << BrianRecord::PresenceOfValidator.new(attribute, opts)
end
end

def valid?
validators.each do |validator|
is_valid = validator.validate(self)
raise validator.message unless is_valid
end
end
end

class PresenceOfValidator
def initialize(attribute, opts={})
@attribute = attribute
end

def message
"expected #{@attribute} to not be nil"
end

def validate(object)
return !object.send(@attribute).nil?
end
end
end


The problem here is that the Captain class lost the validation on parrot attribute which means that you can do the following:

failed scenario
Captain.new(nil, "wood").valid? #=>true, not as intended

The root cause here is that the @validator is only available to a certain class, both Captain and Pirate have their own @validators, so the validate method only iterate thru the @validator available to its own class.
The quick fix Brian provided in the talk was to duplicate the superclass's @validators when initialized the @validators for the subclass, as the following

Framework base class
module BrianRecord
class Base
class << self
def validators
@validators ||= if(superclass.respond_to(:validators)
superclass.validators.dup
else [] end
end
...

This is fine and concise but with only one small glitch, if an validator is added to the superclass after the subclass is loaded, that validator will not be available to the subclass. I brought up another possible solution and we were not sure if it will work. So I didn't a bit coding practice and it looks that the following code also works.


Framework base class
module BrianRecord
class Base

class << self


def validates_presence_of(attribute, opts={})
validators << BrianRecord::PresenceOfValidator.new(attribute, opts)
end

def validate obj
superclass.validate obj if superclass.respond_to?(:validate)
validators.each do |validator|
is_valid = validator.validate(obj)
raise validator.message unless is_valid
end
end

private
def validators
@validators ||= []
end

end

def valid?
self.class.validate self
end
end

In this solution, I move the validate logic to the class, and recursively calls the superclass' validate method if its available. This removed the duplication of the @validators and thus avoid the problem where dynamically added validators will be missing in the subclass. Note here that now that we have a public "validate" class method instead of the public "validators" accessor, which is private now. I would argue that exposing the validate method is better than exposing the validators.

No comments:

Post a Comment