patternMinor
Extend API of complex source-code parsing CPAN module
Viewed 0 times
extendsourcemoduleparsingcodeapicpancomplex
Problem
I am working on a source line parser using
it would be nice if the
Now all these statements contain strings with prefix
How can this best be achieved? In general, the question is how to extend a complex CPAN module like
``
if ( is_elem( $elem, name => 'Token::Operator') ) {
say "Second test ok..";
}
# This last test shows the new method "name":
if ( $elem->name eq 'Toke
PPI. After working a while, I realized that it would be nice if the
PPI API had included such and such functionality. As a very simple example, the PPI::Element contains a class method that returns the class name of the current element. This method must be called quite often and results in statements like if ( $elem->class eq 'PPI::Token::Symbol' ) { ... }Now all these statements contain strings with prefix
PPI::, so it would be nice if there was a an alternative to the class method that would omit the prefix. For exmple a name method:if ( $elem->name eq 'Token::Symbol' ) { ... }How can this best be achieved? In general, the question is how to extend a complex CPAN module like
PPI with user defined methods? I think sub classing the PPI::Element method would not work (see example below) since other PPI methods like find or snext_sibling would still return PPI::Element objects (see below example) and not objects of my sub class. Here is what I came up with: ``
use feature qw(say);
use strict;
use warnings;
BEGIN {
require PPI;
require PPI::Dumper;
require My::PPI::Extensions;
*{PPI::Element::name} = \&My::PPI::Extensions::name;
}
my $line = 'f( $q->{key}[$var + 4] );';
my $doc = PPI::Document->new( \$line );
my $dumper = PPI::Dumper->new( $doc );
$dumper->print;
my $symbols = $doc->find('PPI::Token::Symbol');
my $elem = $symbols->[0];
say "";
say "Start elem: " . $elem->class;
$elem = $elem->snext_sibling;
say "Next elem: " . $elem->class;
# This first test is how it would be done originally,
# here I have to include the "PPI::" prefix
if ( $elem->class eq 'PPI::Token::Operator') {
say "First test ok..";
}
# This second test is to show how it could be done without
# modifying the PPI::Element` APIif ( is_elem( $elem, name => 'Token::Operator') ) {
say "Second test ok..";
}
# This last test shows the new method "name":
if ( $elem->name eq 'Toke
Solution
Now all these statements contain strings with prefix PPI::
I know this is true of all things in PPI itself, and its a very popular convention.
But I suspect there are cases where things may enter the Document without actually being
Also, it seems to me that what you're optimising for is a less frequent usecase than you should be optimising for:
Should be more heavily used in real code than
Either way, you can do better than
You could do
But that's still gross because "you're not using
This could work, but now you're in a bind:
What happens if
These are all, IME, big annoying problems that your desired approach simply makes more complicated to paper over a minor detail.
Which is especially silly when
Will work, on everything, both PPI and not.
That doesn't mean a "name" attribute wouldn't be useful, but it needs to be said that you should not be proposing to use it as the prime deciding mechanism, and should probably only be for diagnostics.
Given all of the above, a functional API is going to be much more to your liking.
The biggest benefit of this approach is there's almost no room for error. There's no chance that one of your objects could be missing the magic methods you require.
Related Reading
I know this is true of all things in PPI itself, and its a very popular convention.
But I suspect there are cases where things may enter the Document without actually being
=~ /^PPI::/.Also, it seems to me that what you're optimising for is a less frequent usecase than you should be optimising for:
$x->isa("PPI::Whatever")Should be more heavily used in real code than
"PPI::Whatever" eq $x->classEither way, you can do better than
"Whatever" eq $x->nameYou could do
$x->is_named('Whatever');But that's still gross because "you're not using
@ISA".$x->is("Whatever")This could work, but now you're in a bind:
What happens if
$x does not subclass whatever class the is method was added on?These are all, IME, big annoying problems that your desired approach simply makes more complicated to paper over a minor detail.
Which is especially silly when
$x->isa("PPI::Whatever")Will work, on everything, both PPI and not.
That doesn't mean a "name" attribute wouldn't be useful, but it needs to be said that you should not be proposing to use it as the prime deciding mechanism, and should probably only be for diagnostics.
Given all of the above, a functional API is going to be much more to your liking.
use PPI::Naming qw( is_named is_an );
# not ISA aware
if ( is_named( $object, 'Whatever' ) ) {
....
}
# ISA aware
if ( is_an( $object, 'Whatever' ) ) {
}is_named would be straight forward:sub is_named {
my ( $object, $name ) = @_;
return undef unless defined blessed( $object );
# This allows somebody to specify "=Whatever" and have it mean
# "Whatever.pm" instead of "PPI::Whatever"
if ( $name =~ /^=(.*$)/ ) {
return $object->class eq "$1";
}
return $object->class eq 'PPI::' . $name;
}is_an would be a little more complicated.use MRO::Compat;
use Scalar::Util qw( blessed );
sub is_an {
my ( $object, $name ) = @_;
my $class = blessed($object);
return undef unless defined $class;
# This allows somebody to specify "=Whatever" and have it mean
# "Whatever.pm" instead of "PPI::Whatever"
if ( $name =~ /^=(.*$)/ ) {
return $object->isa("$1");
}
for my $parent ( mro::get_linear_isa($class) ){
return 1 if $parent eq "PPI::" . $name;
}
}The biggest benefit of this approach is there's almost no room for error. There's no chance that one of your objects could be missing the magic methods you require.
Related Reading
- MRO::Compat
- Scalar::Util
Code Snippets
$x->isa("PPI::Whatever")"PPI::Whatever" eq $x->class"Whatever" eq $x->name$x->is_named('Whatever');$x->is("Whatever")Context
StackExchange Code Review Q#124341, answer score: 5
Revisions (0)
No revisions yet.