Getting loopy while appending HoH

[jedikaiti]

I have this foreach loop that is supposed to grab some info and append it to a hash of a hash that already exists.

foreach $cmd (keys %fssaCmd) {
    $opcode = sprintf("%x",$fssaCmd{$cmd}{fixed_pattern});
    $apid = sprintf("%x",$ApidBase+$ApidOffset);
    print ("$cmd, 0x$apid, 0x$opcode\n");
    %cmdData = (%cmdData, 
        "0x$apid" => {
            opcode => "0x$opcode",
            cmd => $cmd
        }
    )
}

foreach my $a (sort keys %cmdData) {
    print $a.", ".$cmdData{$a}{opcode}.", ".$cmdData{$a}{cmd}."\n";
}

The second foreach loop is something I added to see if the first was working. It's not. The print statement in the first loop is working as expected, indicating that the loop is iterating a total of 6 times. However, the second loop is only printing out 2 lines: what was in the hash to begin with, and the last thing added to it by the first loop.

What am I doing wrong here? Why is everything I append to the hash (except for the last thing) going AWOL?

Many thanks!

[toolic]

Keys to a hash must be unique. $apid is the same value for every iteration through your loop. If you need to support multiple values per key, you could use a Hash-of-Arrays... data structure.

As an aside, Data::Dumper is a convenient way to display your whole data structure:

print Dumper(\%cmdData);

[jedikaiti]

Doh! Crap. *headdesk*

Yea, the Apids are the same. I clearly wasn't thinking when I made those the keys (which I did because that's what I'm going to need to sort by later).

OK, back to the drawing board...

Thanks!

[GrandFather]

First: always use strictures (use strict; use warnings;). $cmd and each of the variables assigned to within the first loop should be declared with my to make it clear that they are local to the loop.

Your 'hash concatenation' trick is horrible! For a better technique have a look at the code below.

As far as I can tell it works for me using some invented data:

use strict;
use warnings;

my %cmdData = map {$_ => {opcode => 1, cmd => 2}} ('a' .. 'b');
my %fssaCmd = map {$_ => {fixed_pattern => 1}} (1 .. 6);
my $ApidBase = 0;
my $ApidOffset = 0;

foreach my $cmd (keys %fssaCmd) {
    my $opcode = sprintf("%x", $fssaCmd{$cmd}{fixed_pattern});
    my $apid   = sprintf("%x", $ApidBase + $ApidOffset);
    print("$cmd, 0x$apid, 0x$opcode\n");
    $cmdData{"0x$apid"} = {opcode => "0x$opcode", cmd => $cmd};
}

foreach my $a (sort keys %cmdData) {
    print $a. ", " . $cmdData{$a}{opcode} . ", " . $cmdData{$a}{cmd} .
+ "\n";
}

Prints:

6, 0x0, 0x1
4, 0x0, 0x1
1, 0x0, 0x1
3, 0x0, 0x1
2, 0x0, 0x1
5, 0x0, 0x1
0x0, 0x1, 5
a, 1, 2
b, 1, 2

Maybe you need to show us a sample of 'real' data that is causing trouble?

[jedikaiti]

When I googled for how to append a perl hash, that's what came up. So I am curious, why is it a "horrible" hash concatenation trick?

[almut]

why is it a "horrible" hash concatenation trick?

It's just that it does way too much unnecessary operations.  To "visualize" what's going on, you could tie the hash and put debugging prints into the routines like STORE, FETCH, etc.  For example:

#!/usr/bin/perl -l

package MyHash;
use Tie::Hash;
our @ISA = 'Tie::StdHash';

sub TIEHASH {
    my $class = shift;
    return bless {}, $class;
}

sub STORE {
    my $self = shift;
    print "storing  $_[0] => $_[1]";
    $self->SUPER::STORE(@_);
}

sub FETCH {
    my $self = shift;
    print "fetching @_";
    $self->SUPER::FETCH(@_);
}

sub CLEAR {
    my $self = shift;
    print "clear";
    $self->SUPER::CLEAR();
}


package main;
use strict;
use warnings;
use Data::Dumper;

tie my %myhash, "MyHash";

print "Variant 1 (bad):";

%myhash = ( %myhash, "key_$_" => { foo => 42 } )  for 1..5;

print "\nDumping...";
print Dumper \%myhash;

%myhash = ();

print "\nVariant 2 (good):";

$myhash{"key_$_"} = { foo => 42 }  for 1..5;

print "\nDumping...";
print Dumper \%myhash;

__END__
Variant 1 (bad):
clear
storing  key_1 => HASH(0x65ecb0)
fetching key_1
clear
storing  key_1 => HASH(0x65ecb0)
storing  key_2 => HASH(0x6c2050)
fetching key_1
fetching key_2
clear
storing  key_1 => HASH(0x65ecb0)
storing  key_2 => HASH(0x6c2050)
storing  key_3 => HASH(0x74a420)
fetching key_3
fetching key_1
fetching key_2
clear
storing  key_3 => HASH(0x74a420)
storing  key_1 => HASH(0x65ecb0)
storing  key_2 => HASH(0x6c2050)
storing  key_4 => HASH(0x74a3f0)
fetching key_3
fetching key_1
fetching key_4
fetching key_2
clear
storing  key_3 => HASH(0x74a420)
storing  key_1 => HASH(0x65ecb0)
storing  key_4 => HASH(0x74a3f0)
storing  key_2 => HASH(0x6c2050)
storing  key_5 => HASH(0x74aaf0)

Dumping...
fetching key_3
fetching key_1
fetching key_5
fetching key_2
fetching key_4
$VAR1 = {
          'key_3' => {
                       'foo' => 42
                     },
          'key_1' => {
                       'foo' => 42
                     },
          'key_5' => {
                       'foo' => 42
                     },
          'key_2' => {
                       'foo' => 42
                     },
          'key_4' => {
                       'foo' => 42
                     }
        };

clear

Variant 2 (good):
storing  key_1 => HASH(0x74ac80)
storing  key_2 => HASH(0x737c40)
storing  key_3 => HASH(0x6c2050)
storing  key_4 => HASH(0x74ad00)
storing  key_5 => HASH(0x74ac00)

Dumping...
fetching key_3
fetching key_1
fetching key_5
fetching key_4
fetching key_2
$VAR1 = {
          'key_3' => {
                       'foo' => 42
                     },
          'key_1' => {
                       'foo' => 42
                     },
          'key_5' => {
                       'foo' => 42
                     },
          'key_4' => {
                       'foo' => 42
                     },
          'key_2' => {
                       'foo' => 42
                     }
        };

[GrandFather]

Almut has provided an excellent answer to why that technique is bad and illustrated why the internet is wonderful. The answer you got through googling illustrates the dark side of the internet. Now, if you'd have asked on PerlMonks ...

[Anonymous Monk]

Your problem is in the parts you've omitted, see How (Not) To Ask A Question