[The following message was posted to the RT GUI interface Sep 20 but was
never copied to the list. So I'm posting it separately here.]
Having pondered the innards of Parrot::Configure
(lib/Parrot/Configure.pm) a lot during recent months, I've had occasion
to wonder why certain parts of it were built the way they were. Because
I'm currently working on two RT tickets that deal with Parrot::Configure
(http://rt.perl.org/rt3//Public/Bug/Display.html?id=45523 and
http://rt.perl.org/rt3//Public/Bug/Display.html?id=45525), I'm trying to
understand the following code from its internal subroutine
_run_this_step():
my $ret; # step return value
eval {
if (@step_params
{
$ret = $step->runstep( $conf, @step_params );
}
else {
$ret = $step->runstep($conf);
}
};
if ($@) {
carp "\nstep $step_name died during execution: [EMAIL PROTECTED]";
return;
}
# did the step return itself?
eval { $ret->can('result'); };
# if not, report the result and return
if ($@) {
my $result = $step->result || 'no result returned';
carp "\nstep $step_name failed: " . $result;
return;
}
my $result = $step->result || 'done';
print "..." if $args->{verbose} && $args->{verbose} == 2;
print "." x ( 71 - length($description) - length($result) );
print "$result." unless $step =~ m{^inter/} && $args->{ask};
This is the code that enables Configure.pl, operating as a harness for
all the configuration steps, to execute the runstep() method from each
individual configuration package (config/*/*.pm). An individual
configuration step is represented by its own object ($step) on which
methods can be called. Most of the $step methods (new(), result(),
etc.) are inherited from Parrot::Configure::Step::Base, but each
configuration step must define its own runstep() method which
encapsulates all the real functionality for the step.
We 'eval' $step->runstep and first rule out the possibility that the step
collapsed during execution.
Then things get curious. Instead of asking, "What was the return value
of $step->runstep()?", we (a) implicitly assume that the return value
from $step->runstep() -- $ret -- is *another* object; and (b) perform
another eval to see whether we can call a 'result' method on that
object. If we cannot, we infer that the configuration step failed to
achieve its objective, print an error message on STDERR, and cause
_run_this_step() to do a bare return, i.e., to return an undefined
value.
And what do we do to get the content of that error message? We go back
to the configuration step object and see if we can get something by
calling a 'result' method on *that* object ($step) -- *not* on $ret.
Failing that, we say 'no result returned'. So although we 'eval' $ret
to see whether we can call a 'result' method on it, we don't actually
*do* anything with the return value of that method.
If we've gotten past these obstacles we conclude that the configuration
step DWIMmed and report that result, again turning to the configuration
step object and its 'result' method to populate our string.
This seems very convoluted to me. The only thing which begins to
explain why this is structured this way is the fact that, by design,
each configuration step *returns its own object* as an indicator of
success. (It returns an undefined value otherwise.) But what this
means is that $ret is the same as, or a descendant of, $step!
Which leads me to wonder:
* Instead of having a configuration step return its own object upon
success, couldn't we simply have it return 1 upon success and bare
return upon failure? ($ret would then always be a simple scalar or
'undef'.)
* If we did so, wouldn't we then be able to get rid of this 'eval':
eval { $ret->can('result'); };
... and then establish what message to print based on the return value
and on whether $step->result has been set within the configuration step?
* In short, why do we mandate that the runstep() methods in the 58
configuration classes return their own objects (return $self)? We don't
do that elsewhere. Is there anything we gain from this that we would
lose by simply having a config step return true upon success?
One final note: docs/configuration.pod says this about the
configuration step object's runstep() method:
"The return value is undefined.
"I<XXX> In the near future the return value of this method will be
significant and there will be a means of passing additional
parameters."
So it seems that the step is *not*, in fact, officially required to
return its own object. This makes the current formulation of
Parrot::Configure::_run_this_step() even more puzzling.
Can anyone shed any light on this? Thank you very much.
kid51
