Developers of Sympa

[IKEDA Soji <address@concealed>]

Guillaume and all,

On Fri, 04 Jul 2014 14:25:20 +0200
Guillaume Rousse <address@concealed> wrote:

> 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.

I don't stick to use of Class::Singleton.  I used it simply he (David)
recommended it (see his posts at 6th and 25th March).
One of pros to use such module may be that use of singleton pattern
will be clearly expressed.

BTW, I looked Logger::* in trunk at a glance, and I was not sure
they generate singleton instances.  Would you like to describe?


> > 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.

I think that only residential (or "ubiquitous"?) objects would be
singleton.  At present, I'm planning to apply such method to three
classes I wrote.

I don't have objection to your argument for now.  I'll write if I
noticed anything.


Regards,

--- Soji

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