[EMAIL PROTECTED] <[EMAIL PROTECTED]> [14-07-2006 12:34]:
> Author: jm
> Date: Fri Jul 14 03:34:29 2006
> New Revision: 421862

> URL: http://svn.apache.org/viewvc?rev=421862&view=rev
> Log:
> bug 4777: evalstr version of run_eval_tests, thanks Michael

> Modified:
>     spamassassin/trunk/lib/Mail/SpamAssassin/PerMsgStatus.pm

Whoa, that's a hackish one.  Wouldn't storing references to anonymous
subs be better (maybe with help of an AUTOLOAD)?  Modifying namespace
at run time is sooo far from OO...  And situation where foo->can("x")
is true, but foo->x() throws an exception...  That's just plain wrong.

[...]
> +  foreach my $method (@TEMPORARY_METHODS) {
> +    if (defined &{$method}) {
> +      undef &{$method};
> +    }
> +  }

Shouldn't it be cleared later?

  @TEMPORARY_METHODS = ();

[...]
> +  # Some of the rules are scoreset specific, so we need additional 
> +  # subroutines to handle those
> +  if (defined &{'Mail::SpamAssassin::PerMsgStatus::'.$methodname}

Prettier (IMHO):

  if (Mail::SpamAssassin::PerMsgStatus->can($methodname)

> +        && !$doing_user_rules)
> +  {
> +    no strict "refs";
> +    
> &{'Mail::SpamAssassin::PerMsgStatus::'.$methodname}($self,@extraevalargs);
> +    use strict "refs";

No need for 'no strict':

  my $method = 'Mail::SpamAssassin::PerMsgStatus::' . $methodname;
  $self->$method(@extraevalargs);

[...]
> +  $evalstr = <<"EOT";
> +{
> +  package Mail::SpamAssassin::PerMsgStatus;
> +
> +    sub ${methodname} {

Less confusing:

  sub Mail::SpamAssassin::PerMsgStatus::$methodname {

[...]

> +    no strict "refs";
> +    
> &{'Mail::SpamAssassin::PerMsgStatus::'.$methodname}($self,@extraevalargs);
> +    use strict "refs";

Same as above.

-- 
Radosław Zieliński <[EMAIL PROTECTED]>

Attachment: pgptOGbAPVf4K.pgp
Description: PGP signature

Reply via email to