All the Perl that's Practical to Extract and Report
use Perl Log In
Ovid (2709)
(email not shown publicly)
http://publius-ovidius.livejournal.com/
AOL IM: ovidperl (Add Buddy, Send Message)
Stuff with the Perl Foundation. A couple of patches in the Perl core. A few CPAN modules. That about sums it up.
More 'iffy' rants
The following code has a bug:
sub _select_fields
{
my $self = shift;
my @fields;
if ($self->request->param('rev_type') eq 'both') {
push @fields, ('rev', 'theater_avg');
} elsif ($self->request->param('rev_type') eq 'total') {
push @fields, 'rev';
} else {
push @fields, 'theater_avg';
}
push @fields, 'num_theaters', 'day_multiple';
return @fields;
}
It turns out that it's supposed to default to "both" instead of its current invisible "loc_avg" default. Further, a new feature has been requested: "day_multiple" should only show up if specifically requested. I certainly didn't want to add to this nasty if/else chain, so I made it a hash. Now it's nice and easy to read.
sub _select_fields
{
my $self = shift;
my %rev_types = (
both => [ qw/rev theater_avg num_theaters/ ],
total => [ qw/rev num_theaters/ ],
loc_avg => [ qw/ theater_avg num_theaters/ ],
all => [ qw/rev theater_avg num_theaters day_multiple/ ],
);
my $rev_type = $self->request->param('rev_type');
$rev_type = 'both' unless exists $rev_types{$rev_type};
return @{$rev_types{$rev_type}};
}
A boolean "if" isn't such a bad thing. It's the nasty if/elsif/else chains that keep cropping up that are problematic. Also note that we've added functionality but the code is shorter and easier to read.
Remember kids: friends don't left friends use "if."
I pour myself a cup of strong coffee. :-)
-- Jarkko Hietaniemi
premature optimisation? (Score:2)
Personally I'd put the initialisation of
%rev_typesoutside the subroutine (but inside anther block to limit the scope) to avoid initialising it each time through the subroutine. But this may well smack of premature optimisation. Then again, it makes it clear that it's a constant structure.Re:premature optimisation? (Score:2)
No, you're right. I should "use constant ..." to make it clear what's going on. Whenever I do a quick refactoring in place, I try to limit the changes. Sometimes that means I wind up not changing enough.
Re:premature optimisation? (Score:1)
But beware. Sometimes that practise bites me when I eventually implement a class, not in it's own
And there is much confusion.
Re:premature optimisation? (Score:1)