Before:
We have a a typical Perl5 if/else chain. The dangling "if" and "die" and at the end should have really be part of the chain, but I did say this was real-world code...
sub _send_headers {
my $self = shift;
my $q = $self->query();
my $header_type = $self->header_type();
if ($header_type eq 'redirect') {
return $q->redirect($self->header_props());
} elsif ($header_type eq 'header' ) {
return $q->header($self->header_props());
}
# croak() if we have an unknown header type
croak ("Invalid header_type '$header_type'") unless ($header_type eq "none");
# Do nothing if header type eq "none".
return "";
}
After:
Of course, "my $self = shift" is gone, (Wahoo!), but notice now the new 'given' switch statement cleans things up nicely:
method _send_headers {
my $q = self.query;
given self.header_type {
when 'redirect' { return $q.redirect(self.header_props) }
when 'header' { return $q.header(self.header_props) }
when 'none' { return "" }
default { die "Invalid header_type '$header_type'" }
}
}
In total: 18 lines reduced to 9, with a clarity gained!
A Perl5 refactoring (Score:1)
sub _send_headers {
my $self = shift;
my $q = $self->query();
my $header_type = $self->header_type();
return
Re:A Perl5 refactoring (winner!) (Score:1)
I commited a variation of this to the CGI::Application p5 source tree.
Re: (Score:1)
More perl 5 refactoring (Score:1)
my $self = shift;
my $q = $self->query;
my %actions = ( redirect => sub { $q->redirect( $self->header_props ) },
header => sub { $q->header($self->header_props()) },
none => sub { return '' },
Re: (Score:1)
Re: (Score:1)