Developers of Sympa

[IKEDA Soji <address@concealed>]

Hi,

On Wed, 26 Feb 2014 10:31:03 +0100
Guillaume Rousse <address@concealed> wrote:

> Le 25/02/2014 11:48, David Verdin a écrit :
> > However, I think it is important to test the presence / absence of
> > expected parameters just after the "my (%params) = @_;" and translate
> > them into local variables. We could even undef the %params hash just
> > after converting its values to local variables.
> > That way, if we had a new key/value to the params hash and try to use it
> > directly in the code a $hash{$key}, we woul find the b_ug at compile time.
> Multiple issues here.
> 
> Testing the presence (and eventually the type) of expected parameters 
> matters, indeed, but I'd restrict it to public functions/methods only. 
> For private functions/methods, that are supposed to be called from the 
> same package only, this is mostly useless, and should be assumed to be 
> done by the caller.
> 
> # this function can be called by any part of sympa
> sub function {
>      my (%params) = @_;
> 
>      # check params first
>      if (!$params{a}) {
>          # handle error
>      }
> 
>      # do something
> }
> 
> # this function can be called only by current package
> sub _function {
>      my (%params) = @_;
> 
>      # assume parameters have been checked by caller
> 
>      # do something
> }

No objection, however, I believe such omission need not be
obliged:  In generall, validation of parameters seems to be
encouraged at any time.

Positional arguments may imply validation on existence of
parameters.

> Second, assuming than copying parameters into local variables always 
> brings additional benefits from compilation-time checks is discussable, 
> especially for short functions, ie:
> 
> sub function {
>      my (%params) = @_;
> 
>      return $params{a} . '@' . $params{b};
> }
> 
> sub function {
>      my (%params) = @_;
> 
>      my $a = $params{a};
>      my $b = $params{b};
> 
>      return $a . '@' . $b;
> }
> 
> The potential typo issue in the actual code of the function in the 
> second form is actually just displaced in the local variable 
> affectation: no safety gain, with two additional variable copies.

I meant of errors on caller side.  If all parameters of all
public interfaces would be named:

1. At first we (implementers) must choose appropriate names;
2. everyone to use interfaces must know all of these names;
3. and they must not use incorrect names.

By each point above, new loads on programming will be added.
For example, hadn't you mistyped the name of parameters named by
yourself, had you?

You may possiblly say such a loads are small to consider.  I don't
agree.  Code of Sympa is open to numerous people.  Generally
speaking, it would be good that unobvious restrictions are reduced.


However, if an interface has many optional parameters, named
parameters may be useful, because programmers may explicitly select
parameters they want to use.

In conclusion, I proposed that mandatory parameters would be
positional arguments and optional ones would be named.


> Third, the ability to reuse initial parameters list directly is very 
> useful in some circunstances, such as wrapping functions with other 
> functions, ie:
> 
> sub generic_function {
>      my (%params) = @_;
>      ...
>      return $something;
> }
> 
> sub less_generic_function {
>      return generic_function(@_, foo => 'value');
> }
> 
> That's exactly the case of current wwslog() function, for instance, that 
> just wraps do_log() function. I wouldn't call it a bug, and even less 
> enforce a mandatory removal of initial parameters list.

Pass-though @_ seems exceptional case for me.

Moreover, it hurts independency of subroutine calls: On second
case, if an element of @_ was modified from inside subfunction,
a variable at outside of main function will be modified.  For
example:

  sub x { $_[0] = 'red'; }

  $color = 'green';
  x($color);
  # Now $color is 'red'.

This is one of reasons why some people insists use of "shift".
If @_ is broken at first in the subroutine, references to variable
on caller-side are broken (See perlfunc(1)).


> Last, I think it is not reasonable to try to enforce a unique 
> fit-them-all rule. Especially with regard to the current code base, the 
> lack of workforce to enforce it, and the difficulties we have to find 
> consensus, even on the smallest cosmetic issues.
> 
> The following example should adress the various concerns, and constitute 
> a suitable *prefered* template for public functions/methods (ie: not the 
> absolute rule):
> 
> sub function {
>      my (%params) = @_;
> 
>      if (!$params{c}) {
>           # log error and return
>      }
> 
>      $a = defined $params{a} ? $params{a} : 'default for A';
>      $b = defined $params{b} ? $params{b} : 'default for B';
>      $c = $params{c);
> 
>      ...
> }
> 
> Is that acceptable for everyone ?

Again, I agree, if named parameters will be used for optional
parameters.


Regards,

--- Soji

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