Developers of Sympa

[IKEDA Soji <address@concealed>]

Guillaume,

On Thu, 23 May 2013 11:01:20 +0200
Guillaume Rousse <address@concealed> wrote:

> Le 21/05/2013 16:05, address@concealed a écrit :
> > --- branches/sympa-6.2-branch/src/lib/Messagespool.pm       2013-05-21 
> > 13:56:37 UTC (rev 9262)
> > +++ branches/sympa-6.2-branch/src/lib/Messagespool.pm       2013-05-21 
> > 14:05:40 UTC (rev 9263)
> > @@ -26,6 +26,14 @@
> >
> >   our @ISA = qw(SympaspoolClassic);
> The names used for those classes doesn't bring any hint about this 
> relationship, and one has to dig in the code to understand it.
> 
> I'd rather suggest:
> SympaSpool for the base class
> SympaSpool::Message for the child class
> 
> And if SympaspoolClassic is meant to be a file-based implementation, 
> this inheritance tree could be even more explicit:
> SympaSpool for the base spool class
> SympaSpool::File for the file-based spool class
> SympaSpool::File::Message for the file-based message spool class
> 
> Of course, the 'Sympa' prefix could also be replaced by a Sympa 
> namespace, as done in the cleanup branch, but that's secondary.

I agree with you on naming convention.  My idea is:

- SympaspoolClassic => Sympa::SpoolFile
- Sympaspool        => Sympa::SpoolSDM

- Messagespool      => Sympa::Spool::Msg
- KeySpool          => Sympa::Spool::Mod
- TaskSpool         => Sympa::Spool::Task (?)

The first two seem to be sibling classes.  Any of child classes
can switch their base in the future.

In addition, I'm considering a few more classes (in these weeks I'm
working offline to refactor codes on interaction between Message object
and spools):

  for 'automatic'      Sympa::Spool::Automatic
  for 'bounce'         Sympa::Spool::Bounce
  for 'digest'         Sympa::Spool::Digest

How is this?


> > +sub new {
> > +    Log::do_log('debug2', '(%s)', @_);
> > +    my $pkg = shift;
> > +    my $spool = SympaspoolClassic->new('msg');
> > +    bless $spool, $pkg;
> > +    return $spool;
> > +}
> That's useless to first bless a reference in its parent class 
> (SympaspoolClassic), and then to bless it again in its final class 
> (Messagespool).
> 
> If the parent class constructor is correct (meaning it doesn't harcode 
> the class to use), this method could be simplified as:
> sub new {
>      Log::do_log('debug2', '(%s)', @_);
>      return $pkg->SUPER::new('msg');
> }

That is what I really wished to do!


Cheers,

-- Soji

> -- 
> Guillaume Rousse
> INRIA, Direction des systèmes d'information
> Domaine de Voluceau
> Rocquencourt - BP 105
> 78153 Le Chesnay
> Tel: 01 39 63 58 31
> 


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