Developers of Sympa

[Guillaume Rousse <address@concealed>]

Le 11/02/2014 01:49, IKEDA Soji a écrit :
[..]
> You may consider using stub module.  For example,
> ---- stub/Conf.pm -----------------------------------
> package Conf;
> our %Conf = (tmpdir => '/path/to/tmp', piddir => '/path/to/piddir');
> ---------------------------------------
> or when Site module is available,
> ---- stub/Site.pm -----------------------------------
> package Site;
> use constant tmpdir => '/path/to/tmp', piddir => '/path/to/piddir';
> ---------------------------------------
> Then run "PERLLIB=stub:src/lib perl sometest.t".
>
> It will pass only the required values, it will not rely on another
> modules, and values will be entirely contollable regardless to
> configurations.  In short, it will serve purpose for unit test.
That's exactly what I refered as 'still testable, but undesirable', for 
the reasons explained previously.

You can't argue for encapsulation on the one hand, and consider global 
variable as desirable on the other hand. Especially without any other 
reason as minimizing changes.

> Mandatory parameter(s) are put on the beginning of argument list
> as unnamed values, then optional parameter(s) follow as named
> values.
>
> sub foo {
>      my $a = shift;
>      my $b = shift;
>      my %params = @_;
>
>      do_something($a, $b, $params{'c'});
> }
Which introduce yet another style, mixing named and positional parameters...

So basically, we have three different style for parameters:
A) positional parameters
sub foo {
   my ($a, $b, $c) = @_;
}
foo($a, $b, $c);

B) named parameters
sub foo {
   my (%params) = @_;
}
foo(arg1 => $a, arg2 => $b, option => $c);

C) mixed parameters
sub foo {
   my ($a, $b, %params) = @_;
}
foo($a, $b, option => $c);

I guess everyone here will agree than enforcing consistent style is 
desirable. Can we have a quick poll on prefered style among other 
developpers ?

I'm strongly in favor of B, as the most self-describing one. Then A 
(simplest), then C.


> (I've already written on use of "shift" and won't repeat it.)
>
> Additional note: if options were passed as hashref, it possibly
> suggest that some kind of object would be passed instead.  You
> would possibly be better to consider another way of refactoring
> (for example, I created a new "User" class).
A class is relevant if there is an underlying semantic, not just to
group an arbitrary list of options.

[..]
> > > Furthermore, though this is the issue once discussed,
> > > removing connection pooling feature from SDM just because it is
> > > complex will obiviously unstabilize Sympa (indeed, it will become
> > > merely unusable).  To "cleanup" unneccessary things is not same as
> > > to remove what one does not understand.
> > I'd rather assert than those kind of 'feature' were introduced to
> > workaround flaky design, but I'd prefer to keep this interesting
> > discussion for a more suitable moment.
>
> Okey, it would be discussed on some other time, but as I have
> written, it seems required feature to handle database connection
> safely: I'd like you to reread past discussion.
I don't think mixing discussion will help at this point.
--
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