As an addendum, I suppose part of my project design is antiquated and I could
simply ditch the ::Interface-provided new_machine() wrapper function entirely,
in which case all implementation code would just be in the implementation, like
all the other wrappers. So then users just say something like:
Class::MOP::load_class( $engine_name );
my $machine = &{$engine_name->can( 'new_machine' )}();
... assuming $engine_name is from a config file, or:
Class::MOP::load_class( $engine_name );
my $machine = $engine_name->new();
... rather than what they do now:
use Muldis::Rosetta::Interface;
my $machine = Muldis::Rosetta::Interface::new_machine({
'engine_name' => $engine_name });
... and so the ::Interface become nothing but "requires foo" roles.
But my main point of this email still stands, about addressing seeming issues of
Class::MOP's load_class().
-- Darren Duncan
Darren Duncan wrote:
Hello,
So I have some Perl code which has evolved over the years, which already
uses Moose, but I'm looking to make it more savvy or modern/enlightened.
And I've also found what appear to be bugs or deficiencies in Class::MOP.
The original code that I'm concerned about updating is a single
subroutine that is copied at the end of this email.
It is the sole content of a Perl "module" Muldis::Rosetta::Interface (as
seen on CPAN), which is analogous in purpose and design to the Perl DBI
module. This non-object function takes a Perl class name as its sole
argument, which is analogous to a DBI driver name, loads that class if
it isn't already loaded, and creates and returns an object of that
class, which is analogous to a DBI driver handle (but that users invoke
its methods directly), and does some tests that the purported
virtual-machine/driver-handle provides the expected API.
Now I recognize that Class::MOP provides these, and other, utility
functions:
Class::MOP::is_class_loaded($class_name)
Class::MOP::load_class($class_name)
... and that these ostensibly are better used instead of doing it manually.
However, from my looking through their source, it appears that these
methods may not be as robust as I might desire.
Part of my question to you is whether what I'm going to describe next is
intentional behavior, or whether it is a candidate for CMOP
enhancement. Or whether I've been expecting behavior that I ought not
to be.
1. I looked for the implementation of is_class_loaded() and it seems
the only implementation is XS, in the file MOP.xs, with no Pure Perl
version existing, although the code comments with that XS says there is
a Pure Perl version in lib/Class/MOP.pm which one should study to better
understand the "grotty logic". Now I interpret this as that the code
comment is outdated and the Pure Perl version was removed sometime
around when it was decided to make a C compiler a mandatory requirement
of using CMOP. If I'm wrong, please point out the Pure Perl version, or
else the comment needs updating.
2. From lib/Class/MOP.pm:
sub _try_load_one_class {
my $class = shift;
return if is_class_loaded($class);
my $file = _class_to_pmfile($class);
return do {
local $@;
local $SIG{__DIE__};
eval { require($file) };
$@;
};
}
One thing that I'm doing which CMOP doesn't seem to do is test after an
error-free eval that the successfully compiled module actually declares
a class of the requested name. For example, by adding another
is_class_loaded() call after the "eval" line iff $@ doesn't hold an
error. So the way CMOP is now, load_class() seems to return true if it
compiles the appropriately named file, and regardless of whether the
desired class was declared by it.
That seems to be an error to me. Would you agree or disagree?
3. I seem to recall reading somewhere, maybe in the Try::Tiny docs,
that it is a common error to test $@ for truthiness rather than
definedness. CMOP is just testing for truthiness, which would I think
fail if someone throws an exception consisting of the number zero or the
empty string.
So that also seems to be an error in CMOP. Agree/disagree?
P.S. Yes, I recognize my own code also has some of the same mistakes of
#3, and I'll fix them.
4. I don't really have any other concerns at the moment about CMOP.
So if load_class() had the semantics I expect, I could then replace half
of my subroutine, the part between START SNIP and END SNIP with:
Class::MOP::load_class( $engine_name )
... wrapped in an eval/try block that enhances the error message.
If any of you have other recommendations for changing my function to
further be Moose savvy or otherwise be more modern/enlightened, I
welcome them.
P.S. While arguably superfluous, the new_machine() function exists at
all because: Like with DBI, the Muldis::Rosetta API has
functions/methods to wrap the actual machine/process/value/etc class
constructor submethods, so that user code doesn't have to be littered
with the names of the actual implementation classes to invoke new() on;
all the users need to know is that the classes do ::Interface roles and
treat them accordingly.
Thank you. -- Darren Duncan
----------
sub new_machine {
my ($args) = @_;
my ($engine_name) = @{$args}{'engine_name'};
confess q{new_machine(): Bad :$engine_name arg; it is undefined}
. q{ or it is the empty string.}
if !defined $engine_name or $engine_name eq q{};
# START SNIP
# A module may be loaded due to it being embedded in a non-excl
file.
if (!do {
no strict 'refs';
defined %{$engine_name . '::'};
}) {
# Note: We have to invoke this 'require' in an eval string
# because we need the bareword semantics, where 'require'
# will munge the module name into file system paths.
eval "require $engine_name;";
if (my $err = $@) {
confess q{new_machine(): Could not load Muldis Rosetta
Engine}
. qq{ module '$engine_name': $err};
}
confess qq{new_machine(): Could not load Muldis Rosetta
Engine mod}
. qq{ '$engine_name': while that file did compile
without}
. q{ errors, it did not declare the same-named module.}
if !do {
no strict 'refs';
defined %{$engine_name . '::'};
};
}
# END SNIP
confess qq{new_machine(): The Muldis Rosetta Engine mod
'$engine_name'}
. q{ does not provide the new_machine() constructor
function.}
if !$engine_name->can( 'new_machine' );
my $machine = eval {
&{$engine_name->can( 'new_machine' )}();
};
if (my $err = $@) {
confess qq{new_machine(): Th Muldis Rosetta Eng mod
'$engine_name'}
. qq{ threw an exception during its new_machine() exec:
$err};
}
confess q{new_machine(): The new_machine() constructor function
of the}
. qq{ Muldis Rosetta Engine mod '$engine_name' did not
ret an}
. q{ obj of a Muldis::Rosetta::Interface::Machine-doing
class.}
if !blessed $machine or !$machine->isa( 'Moose::Object' )
or !$machine->does(
'Muldis::Rosetta::Interface::Machine' );
return $machine;
}
