From: Gavin Sinclair <gsinclair@...>
Date: 2002-08-30T15:29:33+09:00
Subject: Re: Newbie converting brain from perl

>
> Thanks to all who gave advice about the best way to get matching regexp
> items out; I found it very helpful. After years of doing most of my OO
> work  in Java, it's exciting to be reminded that an OO language can be
> as  programmer-friendly as Perl.
>
>
> Here's the next thing that I'm puzzled about.  I'm looking for the most
> efficient way to do a Factory pattern.
>
> When parsing a system log file, there are lots of lines of different
> sorts.  It would make sense that one should have a base class, LogLine,
> and then a  bunch of subclasses for particular entries (SendmailLogLine,
> NamedLogLine,  etc.).
>
> So one might start out with
>
>     class LogLine
>         attr_reader :month, :day, :hour, :minute, :second,
>                 :host, :program, :pid, :message
>         [...]
>     end

I'd use a Time object instead of :month, :day, etc.

>
> But from there, I have two questions:
>
> 1) Is it possible (or even sensible) to override LogLine.new so that it
> returns an instance of the appropriate subclass? I've never liked the
> Java-style LogLine.getInstance(args); it seems unfair to make
> programmers  keep track of whether you're using a Factory internally or
> not.

There's no such method as LogLine.new, I'm afraid.  When you define the
class LogLine, you are creating a constant called LogLine which is of type
class.  When you call LogLine.new, it is the new method in class Class
that you are calling.  It, of course, calls LogLine.init, and the rest is
history.

So creating a (static) method LogLine.create or something like that is
probably the way to go.

I wouldn't try to make subclasses of LogLine straight away, unless you
already know what you want to do with them.

> 2) What's the most efficient way to give each subclass a chance to say
> "Yes,  that line is best represented by me," without doing a lot of
> repetitive  parsing?

Passing the string (minus the date) is all I can think you can do.  You
can't slice the string in a way that's meaningful to all subclasses.

> The obvious OO way seems to be that each subclass would, on loading,
> register itself with LogLine.

Don't know what you mean here.

> As each new LogLine is created, the
> superclass would show the line to each of the subclasses, and whichever
> subclass liked it the best would be instantiated with the data from the
> line.

What does "like it the best" mean?

> Done naively, this would mean that each subclass would parse the line,
> but  that would be wasteful. So obviously, the superclass should do the
> basic  parsing once and pass the details down. Passing as an Array is
> brittle and  not very OO. Passing as a Hash is better, but still not
> right.

It of course depends on your app, but as I said above, you probably can't
produce something at the base level that is meaningful to all subclasses.

BTW There's nothing non-OO about arrays in Ruby.  They're not like Java!

> Really, the parsed data is best represented as a LogLine, no? So it
> would  make sense to instantiate a LogLine with the base data, and then
> pass that  to the subclasses. But when instantiating the subclass, the
> subclass has to  copy over all the data and throw out the copy of the
> superclass, which  seems a little wasteful.

This doesn't seem meaningful to me.  All my comments are pointing to the
conclusion that you're on the wrong track.

> So is it possible for a subclass to somehow replace an instance of its
> superclass with one of its own, stealing its data without explicitly
> copying each field?
>
> I suspect the answer to this is "no", but Ruby's OO is so much more
> flexible  than what I'm used to that I thought it would be good to ask.

Basically, I think you should put all the logic of what subclass best
represents the line into LogLine, like:

class LogLine
  attr_reader :time

  def init; raise "Can't instantiate LogLine"; end

  def create(line)
    index = line[':']
    time = parse_time(line.slice!(index))

    case line
      when SENDMAIL_REGEX
        return SendMailLogLine.new(...)
      when NAMED_REGEX
        return NamedLogLine.new(...)
      else
        return BasicLogLine.new(line)
    end
  end
end

It may be a bit awkward for the client of LogLine to know what to do with
the return value - it'll probably have to type-check it - but that's
because of the subclassing, not because of my approach.

Anyway, you asked for a factory, I gave you a factory!

> Many thanks,
>
> William


Cheers,
Gavin