use Perl

• Stupid question... (Score:1)

  by btilly (5037)

  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.
  • Re: (Score:2)

    by Ovid (2709)

    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
    • Re: (Score:1)

      by btilly (5037)

      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
      • Re: (Score:2)

        by Ovid (2709)

        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
        • Re: Exporter::NoWork (Score:1)

          by mauzo (8185) on 2009.02.03 9:46 (#67188)

          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, and yet again I wish Perl 5 had a 'package name' type, so I didn't have to quote those...)

          Reply to This

          Parent

          • Re: (Score:1)

            by btilly (5037)

            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.
          • Re: (Score:1)

            by phaylon (6115)

            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
            • Re: (Score:2)

              by Ovid (2709)

              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 |
              +---------------+
                ^
                |