Simplifying by refactoring 'array ref' and glob from 'map'

[Matija Papec]

X-Ftn-To: Edward Wijaya

Hi,

My subroutine (filehash_map) below seems to be clumsy,
Would like your advice to make the subroutine more compact?

Regards,
Edward WIJAYA
SINGAPORE

__BEGIN__
#! /usr/bin/perl -w
use strict;

my $dir = 'tmp/*'; #directory to store files
my %documents =();
my %selected_doc =();

my @array = ("file1.txt", "file2.txt");

%documents = &filehash_map($dir);
%selected_doc = &filehash_map(\@array);

^
& has special meaning, so it's better to drop it when you don't need it

perldoc perlsub
===============
To call subroutines:

NAME(LIST); # & is optional with parentheses.
NAME LIST; # Parentheses optional if predeclared/imported.
&NAME(LIST); # Circumvent prototypes.
&NAME; # Makes current @_ visible to called subroutine.

#-----Subroutine--------
sub filehash_map{
# Takes a scalar, check if it's a array reference (list of filename)
# or a scalar (directory that stores files), then create
# hash with filename as key and its content as values

my $dir = shift;
my %documents = ();
my @array = ();

my %documents;
my @array;

you can also drop initialization of hashes and arrays as they are already
empty when declared

if (ref($dir) eq "ARRAY"){
@array = @{$dir};
}
elsif (ref($dir) eq "SCALAR") {
@array = glob($dir);
}
else{
print "Fail - returning empty hash.\n"; #could this error catch be
made better?

You could die here (perldoc -f die) and catch the errror outside of function
with eval,
eval {
%documents = filehash_map($dir);
};
print "There was an eror: $@\n" if $@;

}

%documents = map { $_ => do { local ($/,*F); open F, "<$_"
or die "Error reading '$_': $!"; <F> }} @array;

local ($/,*F);
return map {
$_ => do { open F, $_ or die "Error reading '$_': $!"; <F> }
} @array;

You can pull this "local ($/,*F);" out of the map, and perhaps avoid map
completely in favor to foreach as it may be faster and/or taking less system
memory. If you're into optimization, also consider returning hashref instead
of plain hash as it completely avoids unnecessary copying of hash pairs over
stack.

 

[Edward Wijaya]

Hi,

My subroutine (filehash_map) below seems to be clumsy,
Would like your advice to make the subroutine more compact?

Regards,
Edward WIJAYA
SINGAPORE

__BEGIN__
#! /usr/bin/perl -w
use strict;

my $dir = 'tmp/*'; #directory to store files
my %documents =();
my %selected_doc =();

my @array = ("file1.txt", "file2.txt");

%documents = &filehash_map($dir);
%selected_doc = &filehash_map(\@array);


#-----Subroutine--------
sub filehash_map{
# Takes a scalar, check if it's a array reference (list of filename)
# or a scalar (directory that stores files), then create
# hash with filename as key and its content as values

my $dir = shift;
my %documents = ();
my @array = ();

if (ref($dir) eq "ARRAY"){
@array = @{$dir};
}
elsif (ref($dir) eq "SCALAR") {
@array = glob($dir);
}
else{
print "Fail - returning empty hash.\n"; #could this error catch be
made better?
}

%documents = map { $_ => do { local ($/,*F); open F, "<$_"
or die "Error reading '$_': $!"; <F> }} @array;

return %documents;
}

__END__

 

[Ben Morrow]

local ($/,*F);
return map {
$_ => do { open F, $_ or die "Error reading '$_': $!"; <F> }
} @array;

You can pull this "local ($/,*F);" out of the map,

Or, much better in general, use lexical FHs:

local $/;
return map {
$_ => do {
open my $F, '<', $_ or die "error reading '$_': $!";
<$F>;
}
} @array;

, or, better in this case, use File::Slurp:

use File::Slurp qw/read_file/;

return map { $_ => scalar read_file $_ } @array;

Ben