Developers of Sympa

[IKEDA Soji <address@concealed>]

Hi,

On Fri, 24 May 2013 15:57:35 +0200
Marc Chantreux <address@concealed> wrote:

> hello, 
> 
> On Fri, May 24, 2013 at 02:12:04PM +0200, Guillaume Rousse wrote:
> > >so we can use roles to check that required are implemented
> > I prefer this class hierarchy, which clearly separates concerns.
> 
> i'm not sure about this "this" in this assert :) 
> 
> > However, is there any interest to have some abstraction over spool
> > content (message, key, whatever) ? After all, the calling code
> > usually know which is expected content over each spool...  
> 
> > And have each of the two final classes implement distinct methods
> > for storing distinct kind of content:
> > Sympa::Spool::File::add_message()
> > Sympa::Spool::File::add_key()
> > Sympa::Spool::File::add_something_else()
> 
> It scares me right now as we'll quickly add some 
> 
> * if i have a spool with only 2 methods (say add and delete), i'm happy
>   to see only 2 functions in the code and in the doc.
> * what if you have 2 spool means have incompatible methods: you'll need
>   extra code ? 
> 
> IHMO: Spool provides a basic interface that any spool mean can extend.
> * Spool::Message ISA Spool
> * Spool has Spool::Backend
> * Spool::* contains every business method
> 
> > This doesn't prevent to have mixed-content spool by mistake, but
> > code readability also helps to prevent coding errors :)
> 
> I don't thing splitting things this way would add a lot of complexity
> but you might want to share some set of methods over Spool types: that's
> why the roles are.
> 
> Basically: spools can be queues, trees, ordered or not, thread-safe or
> not, ... maybe it's more that just a backend.
> 
> i'm confused for now. let the week-end help :)

It comes to me that internal data (message content, key and
something else) can be packed into single object.  It will make
things simple: Subclassing of Spool class may not be neccessary.

------------ 8< ---------------- 8< ---------------- 8< ------------
package Spool;

sub new {
    my $pkg = shift;
    my $spoolname = shift;

    if ($spoolname eq 'incoming') {
        return bless {
            'spoolname'  => 'msg',
            'generator'  => 'Message',
            'storagemgr' => 'Backend::File',
        } => $pkg;
    } elsif ($spoolname eq 'pending') {
        return bless {
            'spoolname'  => 'mod',
            'generator'  => 'Message::Keyed', # message with key
            'storagemgr' => 'Backend::SQL',
        } => $pkg;
    } elsif ($spoolname eq 'task') {
        return bless {
            'spoolname'  => 'task',
            'generator'  => 'Task',
            'storagemgr' => 'Backend::File',
        } => $pkg;
    } elsif (...) {

        ...

    }
}

sub get_message {
    my $self      = shift;
    my $selection = shift;

    # fetch() gets a record from physical storage and extract it to
    # hashref.
    # new() generates object from hashref.
    return $self->{'generator'}->new(
        $self->{'storagemgr'}->fetch($self->{'spoolname'}, $selection)
    );
}

sub store {
    my $self   = shift;
    my $object = shift;

    # serialize() is inverse function of new().  It extracts object
    # to hashref.
    # add() adds a record represented by hashref to physical storage.
    $self->{'storagemgr'}->add(
        $self->{'spoolname'},
        $self->{'generator'}->serialize($object)
    );
}

1;
------------ >8 ---------------- >8 ---------------- >8 ------------

It's just an idea.

Regards,

--- Soji

-- 
株式会社 コンバージョン  セキュリティ&OSSソリューション部   池田荘児
〒231-0004　神奈川県横浜市中区元浜町3-21-2 ヘリオス関内ビル7F
e-mail address@concealed  TEL 045-640-3550
http://www.conversion.co.jp/