Factory pattern, abstract base class

Hi all,

I'm trying to setup an abstract base class as a factory for its
subclasses. The below code fails, as it always returns an instance of
Foo::Bar instead of one of its subclasses.

module Foo
   class Bar
      def initialize(arg)
         return if self.class != Foo::Bar
         case arg.downcase
            when 'baz'
               return Baz.new(arg)
            when 'zap'
               return Zap.new(arg)
         end
      end
   end

   class Baz < Bar
   end

   class Zap < Bar
   end
end

fb = Foo::Bar.new('baz')
p fb # Returns Foo::Bar, but I want Foo::Baz

Perhaps my whole approach is wrong. What is the proper way to achieve
what I'm after (without resorting to making Bar a module)?

Thanks,

Dan

Daniel Berger wrote:

Perhaps my whole approach is wrong. What is the proper way to achieve
what I'm after (without resorting to making Bar a module)?
  

The return value from initialize has nothing to do with the object returned from new, which will always be a new instance of the class. To do what you want, define a singleton method not named "new" that returns a new object based on its argument.

This works, but is probably poor style:

module Foo
   class Bar
     def initialize(arg)

     end

     class << self
       alias_method :__new__, :new
       def new(arg)
         return if self != Foo::Bar
         case arg.downcase
         when 'baz'
           return Baz.__new__(arg)
         when 'zap'
           return Zap.__new__(arg)
         end
       end
     end
   end

   class Baz < Bar; end
   class Zap < Bar; end
end

fb = Foo::Bar.new('baz') # => #<Foo::Baz:0x1e2c0c>

__END__

I prefer somthing like this though:

module Foo
   class Bar
     def self.factory(arg)
       return if self != Foo::Bar
       Foo.const_get(arg.capitalize).new(arg)
     end

     def initialize(arg)

     end
   end

   class Baz < Bar; end
   class Zap < Bar; end
end

fb = Foo::Bar.factory('baz') # => #<Foo::Baz:0x1e2f2c>

__END__

Hope that helps.

James Edward Gray II

···

On Dec 2, 2006, at 5:15 PM, Daniel Berger wrote:

Perhaps my whole approach is wrong. What is the proper way to achieve
what I'm after (without resorting to making Bar a module)?

Daniel Berger wrote:

Hi all,

I'm trying to setup an abstract base class as a factory for its
subclasses. The below code fails, as it always returns an instance of
Foo::Bar instead of one of its subclasses.

module Foo
   class Bar
      def initialize(arg)
         return if self.class != Foo::Bar
         case arg.downcase
            when 'baz'
               return Baz.new(arg)
            when 'zap'
               return Zap.new(arg)
         end
      end
   end

   class Baz < Bar
   end

   class Zap < Bar
   end
end

fb = Foo::Bar.new('baz')
p fb # Returns Foo::Bar, but I want Foo::Baz

Perhaps my whole approach is wrong. What is the proper way to achieve
what I'm after (without resorting to making Bar a module)?

module Foo
    class Bar
       def self.new(arg)
          return super() if self != Foo::Bar
          case arg.downcase
             when 'baz'
                return Baz.new(arg)
             when 'zap'
                return Zap.new(arg)
          end
       end
    end

    class Baz < Bar
    end

    class Zap < Bar
    end
end

fb = Foo::Bar.new('baz')
p fb # ==> #<Foo::Baz:0xb7d66c3c>

···

--
       vjoel : Joel VanderWerf : path berkeley edu : 510 665 3407

Daniel Berger wrote:

Hi all,

I'm trying to setup an abstract base class as a factory for its
subclasses. The below code fails, as it always returns an instance of
Foo::Bar instead of one of its subclasses.

module Foo
   class Bar
      def initialize(arg)
         return if self.class != Foo::Bar
         case arg.downcase
            when 'baz'
               return Baz.new(arg)
            when 'zap'
               return Zap.new(arg)
         end
      end
   end

   class Baz < Bar
   end

   class Zap < Bar
   end
end

fb = Foo::Bar.new('baz')
p fb # Returns Foo::Bar, but I want Foo::Baz

Perhaps my whole approach is wrong. What is the proper way to achieve
what I'm after (without resorting to making Bar a module)?

I am more inclined to question your approach than not. After all, if you know the class name (even if it is lowercase) you can use that directly to create instances - even if you do the crude "eval(foo).new" (using const_get is probably better anyway).

My question for you is, what are you eventually trying to achieve? If you want to be able to use some kind of tag as a shortcut for the class to use for instance creation then using a Hash with class instances will be serving you well.

module Foo
   CLASSES = {}

   def self.create(tag, *args,&bl)
     CLASSES[tag].new(*args,&bl)
   end

   class Bar
     CLASSES["baz"] = self
   end
end

Foo.create "baz"

Whether you need that create method and the module at all is probably a different question.

Kind regards

  robert

module Foo
   class Bar
      def self.new(arg)
         return super() if self != Foo::Bar

I don't believe that line does what you intend. Try using your code with something like:

fb = Foo::Baz.new('baz') # => #<Foo::Baz:0x1e2ea0>

         case arg.downcase
            when 'baz'
               return Baz.new(arg)
            when 'zap'
               return Zap.new(arg)
         end
      end
   end

   class Baz < Bar
   end

   class Zap < Bar
   end
end

fb = Foo::Bar.new('baz')
p fb # ==> #<Foo::Baz:0xb7d66c3c>

James Edward Gray II

···

On Dec 2, 2006, at 7:27 PM, Joel VanderWerf wrote:

> Perhaps my whole approach is wrong. What is the proper way to achieve
> what I'm after (without resorting to making Bar a module)?

This works, but is probably poor style:

Which would follow from one of the basic rules of OOP, if memory serves:
A Class does not know anything about its subclasses, be it abstract or not.
OP I suggest that you rethink your design, AFAIK that would not be very
flexible concerning new subclasses right?

<Code snipped>

James Edward Gray II

Cheers

Robert

···

On 12/3/06, James Edward Gray II <james@grayproductions.net> wrote:

On Dec 2, 2006, at 5:15 PM, Daniel Berger wrote:

--
"The real romance is out ahead and yet to come. The computer revolution
hasn't started yet. Don't be misled by the enormous flow of money into bad
defacto standards for unsophisticated buyers using poor adaptations of
incomplete ideas."

- Alan Kay

That's how Factory works in "Design Patterns," and, in fact, in every
implementation I've ever seen. Unless you've run into a limitation of
the pattern itself, this is how I would recommend implementing it.
Apart from anything else, to say that a thing is a subclass of the
factory that creates it is like saying a book is a type of printing
press, or a computer is a type of Taiwan.

You should almost always favor composition over inheritance.

···

On 12/2/06, Timothy Hunter <TimHunter@nc.rr.com> wrote:

To
do what you want, define a singleton method not named "new" that returns
a new object based on its argument.

--
Giles Bowkett
http://www.gilesgoatboy.org

http://gilesgoatboy.blogspot.com

James Edward Gray II wrote:

···

On Dec 2, 2006, at 7:27 PM, Joel VanderWerf wrote:

module Foo
   class Bar
      def self.new(arg)
         return super() if self != Foo::Bar

I don't believe that line does what you intend. Try using your code with something like:

fb = Foo::Baz.new('baz') # => #<Foo::Baz:0x1e2ea0>

It's a little funny to say "baz" twice, is your point?

--
       vjoel : Joel VanderWerf : path berkeley edu : 510 665 3407

Of course every rule has exceptions. For example, in Beck's
Test-Driven Development, after a bit of refactoring you end up with
something like (converted to Ruby :slight_smile:

class Money
  class << self
    def dollar(amount)
      Dollar.new amount
    end

    def franc(amount)
      Franc.new amount
    end
  end
end

The reasoning behind this is that since the clients only go through
Money, you can change the underlying inheritance structure without
breaking anything. Indeed, he goes on to get rid of the Dollar and
Franc classes entirely, and the factory methods turn into:

def dollar(amount)
  new amount, "USD"
end

In the case of the Money class, client code shouldn't have a
dependency on Dollar or Franc classes. Adding a factory method onto
the Money class simplifies client code and promotes refactoring.

It's impossible to tell if OP's situation would be a good use for this
kind of thing, given the lack of explanation and the nondescriptive
names :slight_smile:

I would prefer to have explicit factory methods though, instead of a
single new method that can return an instance of any number of
classes. As far as it not being flexible with subclasses, a little
bit of metaprogramming clears that up:

class Bar
  def self.factory_methods(*names)
    names.each do |n|
      (class << self; self; end).instance_eval do
        define_method(n) { Kernel.const_get(n.to_s.capitalize).send(:new) }
      end
    end
  end
end

class Bar
  factory_methods :baz, :zap
end

Gives you Bar.baz and Bar.zap which return a new Baz and Zap instance,
respectively.

Hell you could even use the inherited callback to do it automatically:

class Bar
  def self.inherited(subclass)
    (class << self; self; end).instance_eval do
      define_method(subclass.to_s.downcase) { subclass.new }
    end
  end
end

Then when you do
class Baz < Bar; end
you automatically get Bar.baz

I'll be honest, I really just saw this whole thing as an opportunity
to do some fun meta-foo :slight_smile: Use at your own risk.

Pat

···

On 12/3/06, Robert Dober <robert.dober@gmail.com> wrote:

On 12/3/06, James Edward Gray II <james@grayproductions.net> wrote:
>
> On Dec 2, 2006, at 5:15 PM, Daniel Berger wrote:
>
> > Perhaps my whole approach is wrong. What is the proper way to achieve
> > what I'm after (without resorting to making Bar a module)?
>
> This works, but is probably poor style:

Which would follow from one of the basic rules of OOP, if memory serves:
A Class does not know anything about its subclasses, be it abstract or not.
OP I suggest that you rethink your design, AFAIK that would not be very
flexible concerning new subclasses right?

btw, check out "Encapsulate Classes with Factory" (p80) in
"Refactoring to Patterns" for a deeper discussion of how and why to
implement this.

Pat

> >
> > > Perhaps my whole approach is wrong. What is the proper way to
achieve
> > > what I'm after (without resorting to making Bar a module)?
> >
> > This works, but is probably poor style:
>
> Which would follow from one of the basic rules of OOP, if memory serves:
> A Class does not know anything about its subclasses, be it abstract or
not.
> OP I suggest that you rethink your design, AFAIK that would not be very
> flexible concerning new subclasses right?

Of course every rule has exceptions.

Hmm I think your mail has shown a flaw in my reasoning, thanks :slight_smile:
My point was that this basic rule explains why James had to come up with
some clumsy (his opinion) code :wink: - and that seems wrong.
Let us imagine, for the sake of argument, that the Rule where not there and
that base classes should now about there subclasses (but not there
implementation details, maybe that is where I was just wrong, now thinking
about it??? SmallTalk seems to implement knowledge about subclasses in the
superclass)

Than one could write
class Factory
      class << self
              subclasses.each do
                      >subclass>
                      define_method subclass.downcase.to_sym { subclass.new}
             end
     end
end

now I think this might not at all have to do with OO design, but with
Retrospection, which might or might not be considered harmful, but is not a
contradiction to the OOP paradigm.
So James' code is complicated because Ruby's introspection does not know
about subclasses, or does it BTW?

For example, in Beck's

Test-Driven Development, after a bit of refactoring you end up with
something like (converted to Ruby :slight_smile:

class Money
  class << self
    def dollar(amount)
      Dollar.new amount
    end

    def franc(amount)
      Franc.new amount
    end
  end
end

The reasoning behind this is that since the clients only go through
Money, you can change the underlying inheritance structure without
breaking anything. Indeed, he goes on to get rid of the Dollar and
Franc classes entirely, and the factory methods turn into:

def dollar(amount)
  new amount, "USD"
end

In the case of the Money class, client code shouldn't have a
dependency on Dollar or Franc classes. Adding a factory method onto
the Money class simplifies client code and promotes refactoring.

It's impossible to tell if OP's situation would be a good use for this
kind of thing, given the lack of explanation and the nondescriptive
names :slight_smile:

I would prefer to have explicit factory methods though, instead of a
single new method that can return an instance of any number of
classes. As far as it not being flexible with subclasses, a little
bit of metaprogramming clears that up:

class Bar
  def self.factory_methods(*names)
    names.each do |n|
      (class << self; self; end).instance_eval do
        define_method(n) { Kernel.const_get(n.to_s.capitalize).send(:new)
}
      end
    end
  end
end

class Bar
  factory_methods :baz, :zap
end

Gives you Bar.baz and Bar.zap which return a new Baz and Zap instance,
respectively.

Hell you could even use the inherited callback to do it automatically:

class Bar
  def self.inherited(subclass)
    (class << self; self; end).instance_eval do
      define_method(subclass.to_s.downcase) { subclass.new }
    end
  end
end

or you could just store the information about the new subclass in a Bar
class instance variable for potential use, that excellent example of your's
mad it clear for me that we were talking Introspection and not OOP.

Then when you do

class Baz < Bar; end
you automatically get Bar.baz

I'll be honest, I really just saw this whole thing as an opportunity
to do some fun meta-foo :slight_smile: Use at your own risk.

Sometimes code is the best reasoning! But it might be a good approach to
think about in that kind of context to use #inherited to enhance the
metadata available. Nice.

Pat

Cheers
Robert

···

On 12/3/06, Pat Maddox <pergesu@gmail.com> wrote:

On 12/3/06, Robert Dober <robert.dober@gmail.com> wrote:
> On 12/3/06, James Edward Gray II <james@grayproductions.net> wrote:
> > On Dec 2, 2006, at 5:15 PM, Daniel Berger wrote:

--
"The real romance is out ahead and yet to come. The computer revolution
hasn't started yet. Don't be misled by the enormous flow of money into bad
defacto standards for unsophisticated buyers using poor adaptations of
incomplete ideas."

- Alan Kay

No. My point was that the original code forces the call to be made on Foo::bar by returning nil otherwise. Your code created the object.

James Edward Gray II

···

On Dec 3, 2006, at 12:04 AM, Joel VanderWerf wrote:

James Edward Gray II wrote:

On Dec 2, 2006, at 7:27 PM, Joel VanderWerf wrote:

module Foo
   class Bar
      def self.new(arg)
         return super() if self != Foo::Bar

I don't believe that line does what you intend. Try using your code with something like:
fb = Foo::Baz.new('baz') # => #<Foo::Baz:0x1e2ea0>

It's a little funny to say "baz" twice, is your point?

In Ruby you can even handle this dynamically. Using the inherited() hook, we could collect a list of available subclasses. That list could then be used to check requested types and instantiate them.

James Edward Gray II

···

On Dec 3, 2006, at 5:02 AM, Pat Maddox wrote:

On 12/3/06, Robert Dober <robert.dober@gmail.com> wrote:

On 12/3/06, James Edward Gray II <james@grayproductions.net> wrote:
>
> On Dec 2, 2006, at 5:15 PM, Daniel Berger wrote:
>
> > Perhaps my whole approach is wrong. What is the proper way to achieve
> > what I'm after (without resorting to making Bar a module)?
>
> This works, but is probably poor style:

Which would follow from one of the basic rules of OOP, if memory serves:
A Class does not know anything about its subclasses, be it abstract or not.
OP I suggest that you rethink your design, AFAIK that would not be very
flexible concerning new subclasses right?

Of course every rule has exceptions. For example, in Beck's
Test-Driven Development, after a bit of refactoring you end up with
something like (converted to Ruby :slight_smile:

class Money
class << self
   def dollar(amount)
     Dollar.new amount
   end

   def franc(amount)
     Franc.new amount
   end
end
end

The reasoning behind this is that since the clients only go through
Money, you can change the underlying inheritance structure without
breaking anything.

James Edward Gray II wrote:

James Edward Gray II wrote:

module Foo
   class Bar
      def self.new(arg)
         return super() if self != Foo::Bar

I don't believe that line does what you intend. Try using your code with something like:
fb = Foo::Baz.new('baz') # => #<Foo::Baz:0x1e2ea0>

It's a little funny to say "baz" twice, is your point?

No. My point was that the original code forces the call to be made on Foo::bar by returning nil otherwise. Your code created the object.

Ah, I see. You are interpreting this line in the original code:

module Foo
    class Bar
       def initialize(arg)
          return if self.class != Foo::Bar # <--- here

as a way of preventing direct instantiation of subclasses (it doesn't actually do that, but maybe that's what was intended). If that was the OP's intent, then I suggest adding protected declarations to what I posted before--see below.

The line

          return super() if self != Foo::Bar

is necessary so that when Bar.new sees the "baz" case and calls Baz.new, which is implemented not in Baz but back up in Bar, the call can be handled by Object.new, which will create a Baz instance and call #initialize on it.

Does that make sense?

Here's the modified code:

   module Foo
      class Bar
         def self.new(arg)
            return super() if self != Foo::Bar
            case arg.downcase
               when 'baz'
                  return Baz.new(arg)
               when 'zap'
                  return Zap.new(arg)
            end
         end
      end

      class Baz < Bar
         class << self; protected :new; end
      end

      class Zap < Bar
         class << self; protected :new; end
      end
   end

   fb = Foo::Bar.new('baz')
   p fb # ==> #<Foo::Baz:0xb7d66c3c>

   Foo::Baz.new("baz") # fails

···

On Dec 3, 2006, at 12:04 AM, Joel VanderWerf wrote:

On Dec 2, 2006, at 7:27 PM, Joel VanderWerf wrote:

--
       vjoel : Joel VanderWerf : path berkeley edu : 510 665 3407

One more suggestion that allows handling of other arguments besides the class selection:

   module Foo
      class Bar
         def self.new(kind, *args)
            return super(*args) if self != Foo::Bar
            case kind.downcase
               when 'baz'
                  return Baz.new(kind, *args)
               when 'zap'
                  return Zap.new(kind, *args)
            end
         end
      end

      class Baz < Bar
         class << self; protected :new; end
         def initialize(*args)
            @args = args
         end
      end

      class Zap < Bar
         class << self; protected :new; end
      end
   end

   fb = Foo::Bar.new('baz', "arg1", 2, [3])
   p fb # ==> #<Foo::Baz:0xb7d5b9b8 @args=["arg1", 2, [3]]>

···

--
       vjoel : Joel VanderWerf : path berkeley edu : 510 665 3407

Yes, I see that now. Took me a surprising amount of time to actually get how you handled that. Very clever.

Still fills wrong to me though, I'm afraid. I guess I feel like we shouldn't muck with Ruby's call semantics for new(), since those are so well known. It makes the results pretty surprising.

At a minimum, I feel the factory method needs a new name.

James Edward Gray II

···

On Dec 3, 2006, at 2:44 PM, Joel VanderWerf wrote:

James Edward Gray II wrote:

On Dec 3, 2006, at 12:04 AM, Joel VanderWerf wrote:

James Edward Gray II wrote:

On Dec 2, 2006, at 7:27 PM, Joel VanderWerf wrote:

module Foo
   class Bar
      def self.new(arg)
         return super() if self != Foo::Bar

I don't believe that line does what you intend. Try using your code with something like:
fb = Foo::Baz.new('baz') # => #<Foo::Baz:0x1e2ea0>

It's a little funny to say "baz" twice, is your point?

No. My point was that the original code forces the call to be made on Foo::bar by returning nil otherwise. Your code created the object.

Ah, I see. You are interpreting this line in the original code:

module Foo
   class Bar
      def initialize(arg)
         return if self.class != Foo::Bar # <--- here

as a way of preventing direct instantiation of subclasses (it doesn't actually do that, but maybe that's what was intended). If that was the OP's intent, then I suggest adding protected declarations to what I posted before--see below.

The line

         return super() if self != Foo::Bar

is necessary so that when Bar.new sees the "baz" case and calls Baz.new, which is implemented not in Baz but back up in Bar, the call can be handled by Object.new, which will create a Baz instance and call #initialize on it.

Does that make sense?