Developers of Sympa

[Guillaume Rousse <address@concealed>]

Le 04/07/2014 08:33, IKEDA Soji a écrit :
> Hi,
>
> On Thu, 03 Jul 2014 19:57:01 +0200
> Guillaume Rousse <address@concealed> wrote:
>
> > Le 21/06/2014 16:10, address@concealed a écrit :
> > > Revision
> > >      11053
> > > Author
> > >      sikeda
> > > Date
> > >      2014-06-21 16:10:49 +0200 (sam. 21 juin 2014)
> > >
> > >
> > >        Log Message
> > >
> > > [svn] Retrieving several modifications on i18n from sympa-6.2-branch.
> > This commit introduces a dependency (Class::Singleton) without any prior
> > discussion about its impact, or its opportunity. For someone so verbose
> > about details such as the semantic difference between a comma and a dash
> > in a copyright statement, or about the need of an executable bit for
> > test argument, this is quite shocking.
> >
> > Moreovoer, I'm not even impressed by the technical relevance of this
> > change, as I don't see much technical advantage of:
> >
> > package Sympa::Anything;
> >
> > my $language = Sympa::Language->instance();
> >
> > over
> >
> > package main;
> >
> > our $language = Sympa::Language->new(
> >       lang => $options{lang} || Sympa::Site->lang()
> > );
> >
> > package Sympa::Anything;
> >
> > my $language = $main::language;
> >
> > The first version allows any part of the code to access a global
> > language instance, even if the top-level didn't explicitely created it,
> > but in this case, it won't be properly initialized from the configuration.
> >
> > The second version is simpler to read, stricter to use, but ensures than
> > incorrect code will fails violently when trying to use a non-existing
> > object.
> >
> > And given than it's the current model I'm trying to generalize in trunk,
> > I'm less than happy to be imposed another one without discussion.
>
> Use of singleton was suggested by David.
> I'm planning to use same method on Site and Logger.
I've no objection against the singleton design pattern, I have an 
objection using Class::Singleton to realize it. Both the logger and the 
mailer are already singletons in the trunk, without any usage of 
additional code.

> Especially on Site, I also consider your suggestion on
> instantiation.
>
> There seems no problem about initialization.
>
> - Instance of Language initially have default setting ('en').
>
> - Instance of Log (Logger) initially outputs all logs to STDERR.
>
> - Instance of Site will load sympa.conf without database config
>    at first instantiation.
Well, you can indeed leverage some ordering constraints during 
initialisation using such a strategy of creating those objects first 
with default values, then reconfiguring them afterward.

Something as:

# process command-line options
my %options;
GetOptions(%options, ...);

# create global objects
our $logger   = Sympa::Logger::Stderr->new();
our $language = Sympa::Language->new();

# read configuration
our $site = Sympa::Site->new($options{config} || '/etc/sympa.conf');

# reconfigure global objects if needed
$logger = Sympa::Logger::Syslog->new(facility => $site->facility());
$language->set_language($options{language} || $site->facility());

However:
- this strategy isn't appliable to all objects: I doubt you can 
instanciate a functioning mailer with default values, for instance
- this have implementation constraints, such as requiring mutable 
objects (for reconfiguration), or preventing to use different classes 
for loggers when using Class::Singleton
- this kind of flexibility won't help reduce the gazillion different way 
to initialize executable already existing in the various executables

I'm more in favour of enforcing stricter initialisation ordering, 
through immutables objets:

# process command-line options
my %options;
GetOptions(%options, ...);

# read configuration
# warning: no logger nor language object available yet
our $site = Sympa::Site->new($options{config} || '/etc/sympa.cong');

# create global objects
our $logger = $proper_logger_class->new(proper options)...
our $language = Sympa::Language->new(
    $options{language} || $site->facility()
);

This has the desadvantage, tough, than any code used for reading the 
configuration should be safe to use without a logger object available.

This ordering issue, tough, is orthogonal to the current issue, and I'm 
less decided about it.

But my original point remains: I'm opposed is to have two different ways 
to manage those global objects, one based on Class::Singleton, one based 
on package variables in main namespace.
--
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