Neil Bowers

NAME

Class::Sniff - Look for class composition code smells

VERSION

Version 0.10

SYNOPSIS

 use Class::Sniff;
 my $sniff = Class::Sniff->new({class => 'Some::class'});

 my $num_methods = $sniff->methods;
 my $num_classes = $sniff->classes;
 my @methods     = $sniff->methods;
 my @classes     = $sniff->classes;

 my $graph    = $sniff->graph;   # Graph::Easy
 my $graphviz = $graph->as_graphviz();
 open my $DOT, '|dot -Tpng -o graph.png' or die("Cannot open pipe to dot: $!");
 print $DOT $graphviz;

 print $sniff->to_string;
 my @unreachable = $sniff->unreachable;
 foreach my $method (@unreachable) {
     print "$method\n";
 }

DESCRIPTION

ALPHA code. You've been warned.

The interface is rather ad-hoc at the moment and is likely to change. After creating a new instance, calling the report method is your best option. You can then visually examine it to look for potential problems:

 my $sniff = Class::Sniff->new({class => 'Some::Class'});
 print $sniff->report;

This module attempts to help programmers find 'code smells' in the object-oriented code. If it reports something, it does not mean that your code is wrong. It just means that you might want to look at your code a little bit more closely to see if you have any problems.

At the present time, we assume Perl's default left-most, depth-first search order. We may alter this in the future (and there's a work-around with the paths method. More on this later).

CLASS METHODS

new

 my $sniff = Class::Sniff->new({
    class  => 'My::Class',
    ignore => qr/^DBIx::Class/,
 });

The constructor accepts a hashref with the following parameters:

  • class (mandatory)

    The name of the class to sniff. If the class is not loaded into memory, the constructor will still work, but nothing will get reported. You must ensure that your class is already loaded!

    If you pass it an instance of a class instead, it will call 'ref' on the class to determine what class to use.

  • ignore (optional)

    This should be a regexp telling Class::Sniff what to ignore in class names. This is useful if you're inheriting from a large framework and don't want to report on it. Be careful with this, though. If you have a complicated inheritance hierarchy and you try to ignore something other than the root, you will likely get bad information returned.

  • universal (optional)

    If present and true, will attempt to include the UNIVERSAL base class. If a class hierarchy is pruned with ignore, UNIVERSAL may not show up.

  • clean (optional)

    If present, will automatically ignore "pseudo-packages" such as those ending in ::SUPER and ::ISA::CACHE. If you have legitimate packages with these names, oops.

  • method_length (optional)

    If present, will set the "maximum length" of a method (lines of code) before it's reported as a code smell. This feature is highly experimental. See long_methods for details.

new_from_namespace

Warning: This can be a very slow method as it needs to exhaustively walk and analyze the symbol table.

 my @sniffs = Class::Sniff->new_from_namespace({
     namespace => $some_root_namespace,
     universal => 1,
 });

 # Print reports for each class
 foreach my $sniff (@sniffs) {
     print $sniff->report;
 }

 # Print out the full inheritance heirarchy.
 my $sniff = pop @sniffs;
 my $graph = $sniff->combine_graphs(@sniffs);

 my $graphviz = $graph->as_graphviz();
 open my $DOT, '|dot -Tpng -o graph.png' or die("Cannot open pipe to dot: $!");
 print $DOT $graphviz;

Given a namespace, returns a list of Class::Sniff objects namespaces which start with the $namespace string. Requires a namespace argument.

If you prefer, you can pass namespace a regexp and it will simply return a list of all namespaces matching that regexp:

 my @sniffs = Class::Sniff->new_from_namespace({
     namespace => qr/Result(?:Set|Source)/,
 });

You can also use this to slurp "everything":

 my @sniffs = Class::Sniff->new_from_namespace({
     namespace => qr/./,
     universal => 1,
 });

Note that because we still pull parents, it's possible that a parent class will have a namespace not matching what you are expecting.

 use Class::Sniff;
 use HTML::TokeParser::Simple;
 my @sniffs = Class::Sniff->new_from_namespace({
     namespace => qr/(?i:tag)/,
 });
 my $graph    = $sniffs[0]->combine_graphs( @sniffs[ 1 .. $#sniffs ] );
 print $graph->as_ascii;
 __END__
 +-------------------------------------------+
 |      HTML::TokeParser::Simple::Token      |
 +-------------------------------------------+
   ^
   |
   |
 +-------------------------------------------+     +---------------------------------------------+
 |   HTML::TokeParser::Simple::Token::Tag    | <-- | HTML::TokeParser::Simple::Token::Tag::Start |
 +-------------------------------------------+     +---------------------------------------------+
   ^
   |
   |
 +-------------------------------------------+
 | HTML::TokeParser::Simple::Token::Tag::End |
 +-------------------------------------------+

All other arguments are passed to the Class::Sniff constructor.

graph_from_namespace

    my $graph = Class::Sniff->graph_from_namespace({
        namespace => qr/^My::Namespace/,
    });
    print $graph->as_ascii;
    my $graphviz = $graph->as_graphviz();
    open my $DOT, '|dot -Tpng -o graph.png' or die("Cannot open pipe to dot: $!");
    print $DOT $graphviz;

Like new_from_namespace, but returns a single Graph::Easy object.

INSTANCE METHODS - CODE SMELLS

overridden

 my $overridden = $sniff->overridden;

This method returns a hash of arrays. Each key is the name of a method in the hierarchy that has been overridden, and the arrays are lists of all classes the method is defined in (not just which one's it's overridden in). The order of the classes is in Perl's default inheritance search order.

Code Smell: overridden methods

Overridden methods are not necessarily a code smell, but you should check them to find out if you've overridden something you didn't expect to override. Accidental overriding of a method can be very hard to debug.

This can also be a sign of bad responsibilities. If you have a long inheritance chain and you override a method in five different levels with five different behaviors, perhaps this behavior should be in its own class.

exported

    my $exported = $sniff->exported;

Returns a hashref of all classes which have subroutines exported into them. The structure is:

 {
     $class1 => {
         $sub1 => $exported_from1,
         $sub2 => $exported_from2,
     },
     $class2 => { ... }
 }

Returns an empty hashref if no exported subs are found.

Code Smell: exported subroutines

Generally speaking, you should not be exporting subroutines into OO code. Quite often this happens when using modules like Carp::croak, which exports subroutines into the use'ing module. These functions may not behave like you expect them to since they're generally not intended to be called as methods.

unreachable

 my @unreachable = $sniff->unreachable;
 for my $method (@unreachable) {
     print "Cannot reach '$method'\n";
 }

Returns a list of fully qualified method names (e.g., 'My::Customer::_short_change') that are unreachable by Perl's normal search inheritance search order. It does this by searching the "paths" returned by the paths method.

Code Smell: unreachable methods

Pretty straight-forward here. If a method is unreachable, it's likely to be dead code. However, you might have a reason for this and maybe you're calling it directly.

paths

 my @paths = $sniff->paths;

 for my $i (0 .. $#paths) {
     my $path = join ' -> ' => @{ $paths[$i] };
     printf "Path #%d is ($path)\n" => $i + 1;
 }

Returns a list of array references. Each array reference is a list of classnames representing the path Perl will take to search for a method. For example, if we have an abstract Animal class and we use diamond inheritance to create an Animal::Platypus class, we might have the following hierarchy:

               Animal
              /      \
    Animal::Duck   Animal::SpareParts
              \      /
          Animal::Platypus

With Perl's normal left-most, depth-first search order, paths will return:

 (
     ['Animal::Platypus', 'Animal::Duck',       'Animal'],
     ['Animal::Platypus', 'Animal::SpareParts', 'Animal'],
 )

If you are using a different MRO (Method Resolution Order) and you know your search order is different, you can pass in a list of "correct" paths, structured as above:

 # Look ma, one hand (er, path)!
 $sniff->paths( 
     ['Animal::Platypus', 'Animal::Duck', 'Animal::SpareParts', 'Animal'],
 );

At the present time, we do no validation of what's passed in. It's just an experimental (and untested) hack.

Code Smell: paths

Multiple inheritance paths are tricky to get right, make it easy to have 'unreachable' methods and have a greater cognitive load on the programmer. For example, if Animal::Duck and Animal::SpareParts both define the same method, Animal::SpareParts' method is likely unreachable. But what if makes a required state change? You now have broken code.

See http://use.perl.org/~Ovid/journal/38373 for a more in-depth explanation.

multiple_inheritance

 my $num_classes = $sniff->multiple_inheritance;
 my @classes     = $sniff->multiple_inheritance;

Returns a list of all classes that inherit from more than one class.

Code Smell: multiple inheritance

See the Code Smell section for paths

duplicate_methods

Note: This method is very experimental and requires the B::Concise module.

 my $num_duplicates = $self->duplicate_methods;
 my @duplicates     = $self->duplicate_methods;

Returns either the number of duplicate methods found, or a list of array refs. Each arrayref contains a list of array references, each having a class name and method name.

Note: 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;
 }

Code Smell: duplicate methods

This is frequently a sign of "cut and paste" code. The duplication should be removed. You may feel OK with this if the duplicated methods are exported "helper" subroutines such as "Carp::croak".

long_methods (highly experimental)

 my $num_long_methods = $sniff->long_methods;
 my %long_methods     = $sniff->long_methods;

Returns methods longer than method_length. This value defaults to 50 (lines) and can be overridden in the constructor (but not later).

  • How to count the length of a method.

     my $start_line = B::svref_2object($coderef)->START->line;
     my $end_line   = B::svref_2object($coderef)->GV->LINE;
     my $method_length = $end_line - $start_line;

    The $start_line returns the line number of the first expression in the subroutine, not the sub foo { ... declaration. The subroutine's declaration actually ends at the ending curly brace, so the following method would be considered 3 lines long, even though you might count it differently:

     sub new {
         # this is our constructor
         my ( $class, $arg_for ) = @_;
         my $self = bless {} => $class;
         return $self;
     }
  • Exported methods

    These are simply ignored because the B modules think they start and end in different packages.

  • Where does it really start?

    If you've taken a reference to a method prior to the declaration of the reference being seen, Perl might report a negative length or simply blow up. We trap that for you and you'll never see those.

Let me know how it works out :)

Code Smell: long methods

Note that long methods may not be a code smell at all. The research in the topic suggests that methods longer than many experienced programmers are comfortable with are, nonetheless, easy to write, understand, and maintain. Take this with a grain of salt. See the book "Code Complete 2" by Microsoft Press for more information on the research. That being said ...

Long methods might be doing too much and should be broken down into smaller methods. They're harder to follow, harder to debug, and if they're doing more than one thing, you might find that you need that functionality elsewhere, but now it's tightly coupled to the long method's behavior. As always, use your judgment.

parents

 # defaults to 'target_class'
 my $num_parents = $sniff->parents;
 my @parents     = $sniff->parents;

 my $num_parents = $sniff->parents('Some::Class');
 my @parents     = $sniff->parents('Some::Class');

In scalar context, lists the number of parents a class has.

In list context, lists the parents a class has.

Code Smell: multiple parens (multiple inheritance)

If a class has more than one parent, you may have unreachable or conflicting methods.

INSTANCE METHODS - REPORTING

report

 print $sniff->report;

Returns a detailed, human readable report of Class::Sniff's analysis of the class. Returns an empty string if no issues found. Sample:

 Report for class: Grandchild
 
 Overridden Methods
 .--------+--------------------------------------------------------------------.
 | Method | Class                                                              |
 +--------+--------------------------------------------------------------------+
 | bar    | Grandchild                                                         |
 |        | Abstract                                                           |
 |        | Child2                                                             |
 | foo    | Grandchild                                                         |
 |        | Child1                                                             |
 |        | Abstract                                                           |
 |        | Child2                                                             |
 '--------+--------------------------------------------------------------------'
 Unreachable Methods
 .--------+--------------------------------------------------------------------.
 | Method | Class                                                              |
 +--------+--------------------------------------------------------------------+
 | bar    | Child2                                                             |
 | foo    | Child2                                                             |
 '--------+--------------------------------------------------------------------'
 Multiple Inheritance
 .------------+----------------------------------------------------------------.
 | Class      | Parents                                                        |
 +------------+----------------------------------------------------------------+
 | Grandchild | Child1                                                         |
 |            | Child2                                                         |
 '------------+----------------------------------------------------------------'

width

 $sniff->width(80);

Set the width of the report. Defaults to 72.

to_string

 print $sniff->to_string;

For debugging, lets you print a string representation of your class hierarchy. Internally this is created by Graph::Easy and I can't figure out how to force it to respect the order in which classes are ordered. Thus, the 'left/right' ordering may be incorrect.

graph

 my $graph = $sniff->graph;

Returns a Graph::Easy representation of the inheritance hierarchy. This is exceptionally useful if you have GraphViz installed.

 my $graph    = $sniff->graph;   # Graph::Easy
 my $graphviz = $graph->as_graphviz();
 open my $DOT, '|dot -Tpng -o graph.png' or die("Cannot open pipe to dot: $!");
 print $DOT $graphviz;

Visual representations of complex hierarchies are worth their weight in gold. See http://pics.livejournal.com/publius_ovidius/pic/00015p9z.

Because I cannot figure out how to force it to respect the 'left/right' ordering of classes, you may need to manually edit the $graphviz data to get this right.

combine_graphs

 my $graph = $sniff->combine_graphs($sniff2, $sniff3);
 print $graph->as_ascii;

Allows you to create a large inheritance hierarchy graph by combining several Class::Sniff instances together.

Returns a Graph::Easy object.

target_class

 my $class = $sniff->target_class;

This is the class you originally asked to sniff.

method_length

 my $method_length = $sniff->method_length;

This is the maximum allowed length of a method before being reported as a code smell. See method_length in the constructor.

ignore

 my $ignore = $sniff->ignore;

This is the regexp provided (if any) to the constructor's ignore parameter.

universal

 my $universal = $sniff->universal;

This is the value provided (if any) to the 'universal' parameter in the constructor. If it's a true value, 'UNIVERSAL' will be added to the hierarchy. If the hierarchy is pruned via 'ignore' and we don't get down that far in the hierarchy, the 'UNIVERSAL' class will not be added.

clean

Returns true if user requested 'clean' classes. This attempts to remove spurious packages from the inheritance tree.

classes

 my $num_classes = $sniff->classes;
 my @classes     = $sniff->classes;

In scalar context, lists the number of classes in the hierarchy.

In list context, lists the classes in the hierarchy, in default search order.

children

 # defaults to 'target_class'
 my $num_children = $sniff->children;
 my @children     = $sniff->children;

 my $num_children = $sniff->children('Some::Class');
 my @children     = $sniff->children('Some::Class');

In scalar context, lists the number of children a class has.

In list context, lists the children a class has.

methods

 # defaults to 'target_class'
 my $num_methods = $sniff->methods;
 my @methods     = $sniff->methods;

 my $num_methods = $sniff->methods('Some::Class');
 my @methods     = $sniff->methods('Some::Class');

In scalar context, lists the number of methods a class has.

In list context, lists the methods a class has.

CAVEATS AND PLANS

  • Package Variables

    User-defined package variables in OO code are a code smell, but with versions of Perl < 5.10, any subroutine also creates a scalar glob entry of the same name, so I've not done a package variable check yet. This will happen in the future (there will be exceptions, such as with @ISA).

  • C3 Support

    I'd like support for alternate method resolution orders. If your classes use C3, you may get erroneous results. See paths for a workaround.

AUTHOR

Curtis "Ovid" Poe, <ovid at cpan.org>

BUGS

Please report any bugs or feature requests to bug-class-sniff at rt.cpan.org, or through the web interface at http://rt.cpan.org/NoAuth/ReportBug.html?Queue=Class-Sniff. I will be notified, and then you'll automatically be notified of progress on your bug as I make changes.

SUPPORT

You can find documentation for this module with the perldoc command.

    perldoc Class::Sniff

You can also look for information at:

ACKNOWLEDGEMENTS

COPYRIGHT & LICENSE

Copyright 2009 Curtis "Ovid" Poe, all rights reserved.

This program is free software; you can redistribute it and/or modify it under the same terms as Perl itself.




Hosting generously
sponsored by Bytemark