factory method in ruby

During my refactoring homework last evening I noticed a little tug-of-war between two different coding styles, and after a restless night I’ll try to analyse here what was going on…

Deep inside Reek is a Source class, whose instances are responsible for converting Ruby code into abstract syntax trees for later examination by the various code smell detectors. Source code can come from many different places, and so Source has a number of Factory Methods like this:

class Source
  def self.from_io(ios)
    new(ios.readlines.join)
  end

  def self.from_s(code)
    new(code)
  end

  def self.from_f(file)
    from_path(file.path)
  end

  def self.from_path(filename)
    new(IO.readlines(filename).join)
  end

  # ...
end

(The factory methods also do some other stuff, which I’ve elided for clarity.)

As the refactoring exercise proceeded, I found that one of these methods came to be called from only one place. So I considered inlining it, and that’s when the tug-of-war began. Part of me resisted inlining the method, even though that’s what the code seemed to want. I argued that the Factory Methods were “potentially useful”, that they represented the Source abstraction and helped to document it; the code pulled the opposite way. (My wife was sitting across the room, so I kept this argument quiet inside my head; maybe that’s part of my problem.) Anyway, to cut a long story short, at the end of my refactoring session the code had changed to (something very similar to) this:

class File
  def to_source
    Reek::Source.new(self.path)
  end
end

class IO
  def to_source
    Reek::Source.new(self.readlines.join)
  end
end

class String
  def to_source
    Reek::Source.new(self)
  end
end

The Gang of Four didn’t name this pattern, so I’ll call these methods “Converters”. (Please tell me if this pattern has a more well-known name.) They’re just like Factory Methods, but they sit on the class being converted from instead of on the class being converted to.

The argument between me and the code was between the forces in favour of each pattern. In favour of the original Factory Methods:

  • In C++/Java — the world in which the GoF did their work, and where my roots lie — this is the only place to put the methods;
  • the dependencies point from Source to its origins (File, String, IO et al), which seems quite natural;
  • the compiler checks that the Factory Methods are called with the right types of input object;
  • all the ways to create a Source object are together, so they are easy to find and compare.

In favour of Converters:

  • We could later introduce new ways to create a Source without opening up that class;
  • they are instance methods, and hence just a little more natural, testable and “object-oriented”;
  • methods such as to_s, to_a, to_i are idiomatic in Ruby, which enhances the Communication value of this approach;
  • you need a File (for example) in order to call the Converter. (Actually, I’m not sure this counts as an advantage, because it prevents duck typing.)

On balance, I feel the C++/Java form of Factory Method has many advantages. But I switched the code to use the Converter form, mainly to see what would happen. I could easily switch back again now I’ve seen the forces laid out for comparison in this blog post…

Advertisements

8 thoughts on “factory method in ruby

  1. Doh! I’ve just figured out the major reason to use Converters: The need to know which parts of the object should be dug out and passed to the Source’s constructor. The example above is trivial in this regard, but one could imagine it being a deal-clincher for something big and hairy that contains “source code” in a non-obvious (and non-public) way.

    (See, this is why the Pomodoro technique doesn’t work for me — I’m just so sloooow…)

    • It’s a good enough name, and now I at least have something to call these. I use C# extension methods to do exactly this sort of thing.

    • Nice, thanks for the link. In my example above, the name-collision problem is another force pushing back towards the Java-style Factory Method approach. I suppose naming the extension methods “to_reek_source” would pretty much avoid that possibility too.

  2. This is actually a common pattern in Smalltalk (which I recall the Gof book also used?) and I think they use the term Converter as well, so your in a good place!

  3. The thing that makes me go hmmm is (still) the way reek accepts both source and pathnames. If this was not the case, you could define #to_source_string on String, File, IO, etc, and have a method like Source.from_source_source – possibly without the homonym abuse. That would factor out the `Reek::Source.new`, but leave the conversion code where it belongs.

    (BTW I’m starting to prefer instance-method factory methods, having used a few lately.)

  4. That’s great, but aren’t you worried about naming conflicts with other gems? You could call the methods ‘to_reek_source’ instead for instance. Or if you’re not worried about this, why not?

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s