CODEWITHSUNDEEP
perlhashmap
Popescu Emanuel
Popescu Emanuel

Reputation: 23

How to assign value to key hashmap Perl?

I have the code bellow. I want to assign for each version values but somehow I do not know what I'm doing wrong.

%buildsMap = ();
#read schedule
opendir (DIR, $buildsDir) or die $!;
while ( my $file = readdir DIR ) {
    if (($file =~ /(V\d\d\d.*)-DVD/) || ($file =~ /(V\d\d\d.*)/)) {
        foreach $version(@versionS){
            if ($1 eq $version ){
               @temp = @{$buildsMap{$version}};
               push @temp,$file;
               @{$buildsMap{$version}} = @temp;
            }
        }
    }
}

If I want to use the keys from this hashmap it is ok. Please advice me.

Upvotes: 1

Views: 284

Answers (1)

Schwern
Schwern

Reputation: 165356

First order of business, turn on strict and warnings. This will discover any typo'd variables and other mistakes. The major issue here will be you have to declare all your variables. Fixing that all up, and printing out the resulting %buildMap, we have a minimum viable example.

use strict;
use warnings;
use v5.10;

my $buildsDir = shift;
my @versionS = qw(V111 V222 V333);
my %buildsMap = ();

#read schedule
opendir (DIR, $buildsDir) or die $!;
while ( my $file = readdir DIR ) {
    if (($file =~ /(V\d\d\d.*)-DVD/) || ($file =~ /(V\d\d\d.*)/)) {
        foreach my $version (@versionS) {
            if ($1 eq $version ){
               my @temp = @{$buildsMap{$version}};
               push @temp,$file;
               @{$buildsMap{$version}} = @temp;
            }
        }
    }
}

for my $version (keys %buildsMap) {
    my $files = $buildsMap{$version};

    say "$version @$files";
}

Which gives the error Can't use an undefined value as an ARRAY reference at test.plx line 15.. That's this line.

            my @temp = @{$buildsMap{$version}};

The problem here is how you're working with array references. That specific line fails because if $buildsMap{$version} has no entry you're trying to dereference nothing and Perl won't allow that, not for a normal dereference. We could fix that, but there's better ways to work with hashes of lists. Here's what you have.

           my @temp = @{$buildsMap{$version}};
           push @temp,$file;
           @{$buildsMap{$version}} = @temp;

That copies out all the filenames into @temp, works with @temp and the more comfortable syntax, and copies them back in. It's inefficient for memory and amount of code. Instead, we can do it in place. First by initializing the value to an empty array reference, if necessary, and then by pushing the file onto that array reference directly.

            $buildsMap{$version} ||= [];
            push @{$buildsMap{$version}}, $file;

||= is the or-equals operator. It will only do the assignment if the left-hand side is false. It's often used to set default values. You could also write $buildsMap{$version} = [] if !$buildsMap{$version} but that rapidly gets redundant.

But we don't even need to do that! push is a special case. For convenience, you can dereference an empty value and pass it to push! So we don't need the initializer, just push.

            push @{$buildsMap{$version}}, $file;

While the code works, it could be made more efficient. Instead of scanning @versionS for every filename, potentially wasteful if there's a lot of files or a lot of versions, you can use a hash. Now there's no inner loop needed.

my %versionS = ( 'V111' => 1, 'V222' => 2, 'V333' => 3 );

...

if (($file =~ /(V\d\d\d.*)-DVD/) || ($file =~ /(V\d\d\d.*)/)) {
    my $version = $1;
    if ($versionS{$version}){
        push @{$buildsMap{$version}}, $file;
    }
}

Upvotes: 3

Related Questions