Developers of Sympa

[Guillaume Rousse <address@concealed>]

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.

> +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');
}

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

Attachment: smime.p7s
Description: Signature cryptographique S/MIME