chromatic wrote:
On Saturday 14 July 2007 09:05:02 [EMAIL PROTECTED] wrote:
Author: paultcochrane
Date: Sat Jul 14 09:05:01 2007
New Revision: 19863
Modified:
trunk/lib/Parrot/Configure.pm
trunk/lib/Parrot/Configure/Data.pm
trunk/lib/Parrot/Configure/Step.pm
Log:
[lib] Using explicit returns as per coding standards
I don't trust some of these, and it makes me question the utility of this
coding standard test.
I agree. I haven't done a thorough check, but I've already spotted at
least one case, in lib/Parrot/Configure.pm, where addition of 'return'
changed the behavior of the function.
###################
sub runstep {
my $self = shift;
my $taskname = shift;
my ( $verbose, $verbose_step, $ask ) =
$self->options->get( qw( verbose verbose-step ask ) );
for my $task ( $self->steps() ) {
if ( $task->{"Parrot::Configure::Task::step"} eq $taskname ) {
$self->_run_this_step( {
task => $task,
verbose => $verbose,
verbose_step => $verbose_step,
ask => $ask,
n => 1,
} );
}
}
return; # <-- recently added -- and erroneous
}
###################
Until the addition of that 'return' statement, the return value of the
method would have been the truth value of the last statement evaluated
within the method -- most likely, the evaluation of the last iteration
of _run_this_step, but (if the string equality were not true) possibly
the evaluation of the get() call earlier on. In either case, the
evaluation of the last statement and, hence, the return value of the
subroutine was overwhelmingly likely to be a true value.
A bare return means that the method now evaluates to 'undef', a false value.
The kicker is that this would have been very difficult to detect. We
(or I) haven't yet managed to write explicit unit tests for
Parrot::Configure::runstep() because it's only used in
*post-configuration* development setups by people like pmichaud and
particle. It's not part of the regular configure-build-test cycle.
In the modules I have worked on over the past nine months, including:
Parrot::Pmc2c::Utils (now Parrot::Pmc2c::Pmc2cMain)
Parrot::Ops2pm::Utils
Parrot::Ops2c::Utils
Parrot::Configure
Parrot::Configure::Data
Parrot::Configure::Options
Parrot::Configure::Options::Test
Parrot::Configure::Step
Parrot::Configure::Step::List
Parrot::BuildUtil
Parrot::Revision
... you can be certain that if I did *not* type an explicit 'return'
statement at the end of a subroutine, that omission was *deliberate*.
In particular, I generally don't have subroutines end with a bare
'return'. If a subroutine DWIMs, then I consider its result to be
"true" and I want it to return a value which Perl evaluates to be a true
value.
This, after all, is Perl, where 1 is true and 0/undef/q{} is false.
Only when our programs need to signal their success or failure to the
outside world or receive signals from that world ('system') do we revert
to the Unix convention of 1 being false and 0 being true.
kid51
