Developers of Sympa

[IKEDA Soji <address@concealed>]

I don't agree.

I feel that breaking encapsulation for convenience of testing
is mistaking means for ends.

Anyway, I feel regretful that I don't have enough time nor will to
discuss on this issue right now.

Regrads,

--- Soji

On Tue, 24 Jun 2014 09:15:02 +0200
Guillaume Rousse <address@concealed> wrote:

> Le 23/06/2014 18:53, IKEDA, Soji a écrit :
> > Guillaume,
> >
> > I don't agree to this method.
> You're supposed to provide technical argumentation to backup your point...
> 
> The previous code is difficult to test, without relying on non-trivial 
> techniques such as preloading a mock Site class, and rewriting it on the 
> fly to chain multiple tests with different parameters. The new code, 
> making the method self-contained, allows to chain arbitrary tests 
> without troubles.
> 
> More generally, having each part of Sympa code directly access 
> configuration has multiple drawbacks:
> - circular dependencies between packages
> - strong components coupling
> - lack of internal compartimentation
> 
> Modularity does not means moving code around in different files, it's 
> also ensuring that each part if independant enough to be potentially 
> replaced by something else. There is no way to replace parts of Sympa 
> specific code with CPAN alternative as long as those parts have so much 
> intimity with Sympa internals. And for me the target design is clear: 
> only the top-level code should access the configuration directly, not 
> the objects modeling the application logic.
> 
> Basically, that's about switching from this model:
> 
> package main;
> 
> my $object = Sympa::Object->new();
> $object->do_something();
> 
> package Sympa::Object;
> 
> # configuration access needed
> use Sympa::Site;
> 
> # no parameter, because of direct configuration access
> sub do_something {
> }
> 
> to this model:
> 
> package main;
> 
> # configuration access needed
> use Sympa::Site;
> 
> my $object = Sympa::Object->new();
> $object->do_something(foo => Sympa::Site->foo());
> 
> package Sympa::Object;
> 
> # no dependency
> 
> # parameters needed, because the method is self-contained
> sub do_something {
>      my (%params) = @_;
> 
>      my $foo = $params{foo};
>      croak "missing parameter foo" unless $params{foo};
> }
> 
> In the second model, the do_something() method needs are explicit, 
> whereas they are hidden in the first. And the day where the 
> configuration parameter name change, or the way to access it from the 
> configuration change, there is no need to change anything in Sympa::Object.
> -- 
> 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/