Refactoring code snippet
Recently I encountered a Perl script that had some issues. e.g. Lack of use strict.
Let me show a quick refactoring of it:
Part of the original code
examples/code_snippet0.pl
my $copyrightsfile = shift @ARGV; open COPYR,"<$copyrightsfile" or die "Can't open copyrights $copyrightsfile"; while(<COPYR>){ chomp; next if /^#/; @COPYRIGHT=split /,/; next unless scalar(@COPYRIGHT)==2; push @COPYRIGHTS,{ firstname=>$COPYRIGHT[0], lastname=>$COPYRIGHT[1], }; } close COPYR; use Data::Dumper qw(Dumper); print Dumper \@COPYRIGHTS;
There are some interesting things here:
- missing use strict
- But declaring the global variable $copyrightsfile using my
- Not declaring @COPYRIGHTS (in plural) at all even though it is a global variable.
- Not declaring @COPYRIGHT (in singular) either, even though it should be locally scoped. And the confusion of having two arrays with very similar names.
- Using the implicit $_ might be considered "perlish" by some, but I prefer to have a named variable in its place.
- I addedd the Data::Dumper code to make it easier to see the results.
Sample imput file
examples/code_snippet.txt
Jane,Doe Joe, Mary
Result:
$VAR1 = [ { 'firstname' => 'Jane', 'lastname' => 'Doe' } ];
Add use strict, Use lexical file-handle
Lexical file-handles and 3-part open.
That forces us to declare our varibales with my.
examples/code_snippet1.pl
use strict; use warnings; my $copyrightsfile = shift @ARGV; my @COPYRIGHTS; open my $COPYR, '<', $copyrightsfile or die "Can't open copyrights $copyrightsfile"; while(<$COPYR>){ chomp; next if /^#/; my @COPYRIGHT=split /,/; next unless scalar(@COPYRIGHT)==2; push @COPYRIGHTS,{ firstname=>$COPYRIGHT[0], lastname=>$COPYRIGHT[1], }; } close $COPYR; use Data::Dumper qw(Dumper); print Dumper \@COPYRIGHTS;
Replace @COPYRIGHT, by $firstname, $lastname
examples/code_snippet2.pl
use strict; use warnings; my $copyrightsfile = shift @ARGV; my @COPYRIGHTS; open my $COPYR, '<', $copyrightsfile or die "Can't open copyrights $copyrightsfile"; while(<$COPYR>){ chomp; next if /^#/; my ($firstname, $lastname) = split /,/; next if not defined $lastname or $lastname eq ''; push @COPYRIGHTS,{ firstname => $firstname, lastname => $lastname, }; } close $COPYR; use Data::Dumper qw(Dumper); print Dumper \@COPYRIGHTS;
Use explicit $line instead of $_ in the while loop
examples/code_snippet3.pl
use strict; use warnings; my $copyrightsfile = shift @ARGV; my @COPYRIGHTS; open my $COPYR, '<', $copyrightsfile or die "Can't open copyrights $copyrightsfile"; while(my $line = <$COPYR>){ chomp $line; next if $line =~ /^#/; my ($firstname, $lastname) = split /,/, $line; next if not defined $lastname or $lastname eq ''; push @COPYRIGHTS,{ firstname => $firstname, lastname => $lastname, }; } close $COPYR; use Data::Dumper qw(Dumper); print Dumper \@COPYRIGHTS;
Published on 2021-01-13
If you have any comments or questions, feel free to post them on the source of this page in GitHub. Source on GitHub.
Comment on this post