Stories
Slash Boxes
Comments
NOTE: use Perl; is on undef hiatus. You can read content, but you can't post it. More info will be forthcoming forthcomingly.

All the Perl that's Practical to Extract and Report

use Perl Log In

Log In

[ Create a new account ]

Ovid (2709)

Ovid
  (email not shown publicly)
http://publius-ovidius.livejournal.com/
AOL IM: ovidperl (Add Buddy, Send Message)

Stuff with the Perl Foundation. A couple of patches in the Perl core. A few CPAN modules. That about sums it up.

Journal of Ovid (2709)

Monday February 02, 2009
03:58 PM

Class::Sniff Now Detects "Duplicate" Methods

[ #38395 ]

Yeah, I know, you're sick of hearing about Class::Sniff. I'll be over this manic streak soon :)

It now detects and reports "duplicate" methods via this evil hack:

my $walker = B::Concise::compile( '-terse', $coderef );
B::Concise::walk_output( \my $buffer);
$walker->();
$buffer =~ s/^.*//;   # strip method name
$buffer =~ s/\(0x[^)]+\)/(0xHEXNUMBER)/g;   # normalize addresses
my $digest = Digest::MD5::md5_hex($buffer);
$self->{duplicates}{$digest} ||= [];

# later we check if there is more than one entry
push @{ $self->{duplicates}{$digest} } => [$class, $method];

It's severely limited, but I'm hoping it will offer a slight benefit for obviously cut-n-pasted code. We report duplicates based on identical op-trees. If the method names are different or the variable names are different, that's OK. Any change to the op-tree, however, will break this. The following two methods are identical, even if they are in different packages:

sub inc {
    my ( $self, $value ) = @_;
    return $value + 1;
}

sub increment {
    my ( $proto, $number ) = @_;
    return $number + 1;
}

However, this will not match the above methods:

sub increment {
    my ( $proto, $number ) = @_;
    return 1 + $number;
}

Class::Sniff 0.05 should be at your local CPAN mirror soon.

(Now to get around to the Foo::Bar, Foo::Bar::Baz issue that Adam Kennedy pointed out).


List all Journal entries
The Fine Print: The following comments are owned by whoever posted them. We are not responsible for them in any way.

Class::Sniff Now Detects "Duplicate" Methods 11 Comments More | Login | Reply /

 Full
 Abbreviated
 Hidden
More | Login | Reply
Loading... please wait.

  • If two packages import the same function from a module, will it think that this is duplicated code in 3 packages, or will it notice that they are the same subroutine so one function happens to appear in 3 places?

    If it sees this as duplicated code then I foresee enough false positives to render it useless.

    • It will think this is duplicated code in 3 packages. You would be running this on OO code, generally. Exporting functions into OO packages is a dodgy practice. It can override things you didn't want it to override and can show up as a "method" when it shouldn't. However, that's why I have the human-readable report and these are just "smells". After a while, people will learn to recognize these issues and ignore them.

      I might even provide a .sniffrc format so that people can ignore Carp::croak or whateve

      • Well as you say, lots of OO code still imports croak. Also there are people who do mixins. What about those? Sure, you can claim they are dodgy, but when they happen fairly often not by accident.

        It seems to me that if you know that A::foo and B::bar have the same opcode, that it wouldn't be too hard to check whether \&A::foo == \&B::foo before reporting on the problem? For simple OO code this is would cut down the false positives. That eliminates the repeated croak issue fairly easily.

        Unfortuna

        • Ironically, lots of getters and setters are also a code smell in OO code, but trying to explain that one to programmers is like trying to nail pudding to a wall. As for mixins, traits and roles, yes, this will be problematic. I don't want special cases, but I think I can handle Class::Trait [cpan.org] and Moose::Role [cpan.org], as I think those are the two most common ways of implementing this.

          The \&A::foo == \&B::foo is important, but there's an interesting caveat. For a couple of the unreachable methods I discovere

          • Would an option to not re-export subs which were imported from somewhere else help, or are you relying on that behaviour?

            I often wish there was a straightforward way to import subs into a lexical scope, so you can use unqualified function names without them showing up as methods. Moose offers no Moose; to do this, but it would be good to have something that hooked into UNITCHECK or some such which would allow

                use lexically "Carp";
                use lexically "Scalar::Util", qw/weaken/;

            (Oh, an

            • The straightforward way you were not looking for is:

              my $weaken = \&Scalar::Util::weaken; ...
              $weaken->($ref);

              It looks odd, but I've actually used this technique upon rare occasion and found it worked well.

            • I do this all the time instead:

              package Foo;
              use Moose; # exports a lot implicitly
              use Scalar::Util qw( blessed );

              # this removes everything imported above, but
              # expcludes the meta method installed by Moose
              use namespace::clean -except => 'meta';

              --
              Ordinary morality is for ordinary people. -- Aleister Crowley

              • I know you know this, but for others: namespace::clean is great. It removes those methods from the namespaces. Thus:

                #!/usr/bin/env perl

                use strict;
                use warnings;

                {
                    package Foo;
                    use Moose;
                    use Scalar::Util 'blessed';
                    use namespace::clean -except => 'meta';
                }
                use Class::Sniff;
                my $sniff = Class::Sniff->new({class => 'Foo'});
                print $sniff->to_string;
                print $sniff->report;

                __END__
                +---------------+
                | Moose::Object |
                +---------------+
                  ^
                  |

          • Yes, I am well aware of the theories of OO design which say you don't want getters and setters. However I am also well aware that there are plenty of common approaches to common problems that use getters and setters and work well. Being the kind of person who prefers to worry about practicalities rather than someone's abstract theories, I feel no need to apologize about using getters and setters where it is appropriate.

            An example I would offer is for using an object-relational mapper like Rose::DB to make

  • Yeah, I know, you're sick of hearing about Class::Sniff

    I'm not! It's good to hear about useful stuff that people are working on.

Stories, comments, journals, and other submissions on use Perl; are Copyright 1998-2006, their respective owners.