[Rt-devel] Why two IsRTAddress() and CullRTAddresses()?

[Ruslan Zakirov]

Check out 3.6.3RC2, there is some changes in these functions, but
still we should get rid of all duplicates.

On 12/19/06, Petter Reinholdtsen <pere at hungry.com> wrote:
>
> In RT 3.6.1 there seem to be two functions using the
> $RT::RTAddressRegexp regular expression.  Both
> RT::EmailParser::IsRTAddress() and RT::Interface::Email::IsRTAddress()
> exists.  Why?  The latter do not seem to be used.  Should it be
> deleted?
>
> I'm looking into rewriting it to get it to look up addresses in LDAP
> to check if they are forwarded to the RT server, and found it strange
> that I need to override two functions to do this.  CullRTAddresses()
> is duplicated as well.  The only functional difference seem to be that
> the ones in EmailParser.pm take $self as the first argument and the
> one in Email.pm accept undef.
>
> lib/RT/EmailParser.pm:
>
>     sub IsRTAddress {
>         my $self = shift;
>         my $address = shift;
>
>         # Example: the following rule would tell RT not to Cc
>         #   "tickets at noc.example.com"
>         if ( defined($RT::RTAddressRegexp) &&
>                            $address =~ /$RT::RTAddressRegexp/i ) {
>             return(1);
>         } else {
>             return (undef);
>         }
>     }
>     sub CullRTAddresses {
>         my $self = shift;
>         my @addresses= (@_);
>         my @addrlist;
>
>         foreach my $addr( @addresses ) {
>                                      # We use the class instead of the instance
>                                      # because sloppy code calls this method
>                                      # without a $self
>           push (@addrlist, $addr)    unless RT::EmailParser->IsRTAddress($addr);
>         }
>         return (@addrlist);
>     }
>
>
> lib/RT/Interface/Email.pm:
>
>     sub IsRTAddress {
>         my $address = shift || '';
>
>         # Example: the following rule would tell RT not to Cc
>         #   "tickets at noc.example.com"
>         if ( defined($RT::RTAddressRegexp)
>             && $address =~ /$RT::RTAddressRegexp/i )
>         {
>             return (1);
>         } else {
>             return (undef);
>         }
>     }
>
>     sub CullRTAddresses {
>         return ( grep { IsRTAddress($_) } @_ );
>     }
>
> The CullRTAddresses() version in lib/RT/Interface/Email.pm is more
> elegant than the one in lib/RT/EmailParser.pm.  I suggest to rewrite
> them like this:
>
> lib/RT/EmailParser.pm:
>
>     sub IsRTAddress {
>     [no change]
>     }
>     sub CullRTAddresses {
>         my $self = shift;
>
>         # We use the class instead of the instance because sloppy code
>         # calls this method without a $self
>         return ( grep { RT::EmailParser->IsRTAddress($_) } @_ );
>     }
>
> lib/RT/Interface/Email.pm (or just remove them because they are
> unused):
>
>     sub IsRTAddress {
>         my $address = shift || '';
>         return RT:EmailParser->IsRTAddress($address);
>     }
>
>     sub CullRTAddresses {
>         return RT:EmailParser->CullRTAddresses(@_);
>     }
>
> Is this a good idea?
>
> Friendly,
> --
> Petter Reinholdtsen
> _______________________________________________
> List info: http://lists.bestpractical.com/cgi-bin/mailman/listinfo/rt-devel
>


-- 
Best regards, Ruslan.