Fail to update an array in HoAoA

[Anonymous Monk]

Master,
I am a newcomer for Perl. Please kindly bear with me. Why my code below:

#!/usr/bin/perl -w
use strict;
use Data::Dumper;

my $key;
my %hoa;

while ( <DATA> )
{
   my @aoa;
   next if (/^\s+/);

   if ( /^SET/ )
   {
     my @KEY = split (/[\s:]/,$_);
     $key = $KEY[2];
   }
   elsif (/^\d/)
   {
       my @INST = split(/,/,$_);
       push @aoa, [ @INST ];
   }

       $hoa{$key} = [@aoa];
}
       print Dumper \%hoa ;

__DATA__
SET: SET1

0,100,BOOK
1,150,PENCIL
====

SET: SET2

2,110,ERASER
2,200,PEN
0,220,BLACKBOARD
1,300,CHALK
====

Fail to give this:

$VAR1 = {
          'SET1' => [
                      [ '0','100','BOOK'],
                      [ '1','150','PENCIL'],
                    ],
          'SET2' => [
                      [ '2','110','ERASER'],
                      [ '2','200','PEN'],
                      [ '0','220','BLACKBOARD'],
                      [ '1','300','CHALK'],
                    ]
        };

Thanks so much for your patience.

[Fang]

There are multiple logical design errors in the flow of your program. One of them is that you assign an arrayref to $hoa{$key} while you want it to be an arrayref itself, so you have to push your data in that array like push @{hoa{$key}}, [ @aoa ]. This assignment is also misplaced, as it will push an empty arrayref in case the line from <DATA> doesn't match any of the pattern you mentionned (/^\s+/ won't match an empty line by the way, /^$/ will).

Here's some code that will produce what you want. You may look at that solution (among others, as always) directly, or try and correct your code a bit by yourself first.

Read more... (709 Bytes)

Update: updated explanations a bit, small updates to the code.

[Tanalis]

This seems a little homeworky to me, so I'm not going to just write some code for you. I'll point out a few areas I think you should look at to get a little closer to where you need to be.

Take a closer look at the line

my @KEY = split (/[\s:]/,$_);

If you're trying to split the line on ': ' (colon space), I don't think you want to be using a character class: simply the literal ': ' will do:

my @KEY = split( /: /, $_ );

For data like this set, you could completely replace this split with a regular expression, avoiding the use of the intermediate arrays.

Secondly, think about your @aoa array. As you have it at the minute, it's being redefined each time the while loop iterates. Is that what you need to happen?

Finally, think about your assignment to the %hoa. It's going to the array you have so far to the hash each time the loop iterates - which doesn't seem healthy to me.

I'd consider making use of a second loop, something like this:

for each data line
{
   grab the key

   loop
     assign data rows to the array of arrays
   until there's no more data

   assign array of arrays to hash
}

Using an inner loop in this way will allow you to read the key separately to reading the data, meaning that you can build a complete array of arrays before assigning it to the hash of arrays. This also helps to make your code more readable (in my opinion), and hence more maintainable.

Hope that helps you a little.

[tphyahoo]

UPDATE: Wait a minute, that doesn't quite work.
UPDATE 2: Okay, works now. That first sentence in my original post should have been: "Take @arr out of the while loop, clear @arr every time you get a new key, and you're good to go."

**********

Get rid of @arr out of the while loop, clear @arr every time you get a new key, and you're good to go.

You might also want to change @INST and @KEY to @inst and @key, since by convention capital variable names are constants: perlstyle. (Some people also use capitals to indicate globals, though pelstyle suggests using mixed case to do this.)

Read more... (929 Bytes)

[tphyahoo]

As an alternative to using Data::Dumper in debugging, I have begun using Test::More::is_deeply. The more I try it, the more I am beginning to like it... :)

#!/usr/bin/perl -w
use strict;
use warnings;
use Test::More qw(no_plan);

my $expected = {
          'SET1' => [
                      [ '0','100','BOOK'],
                      [ '1','150','PENCIL'],
                    ],
          'SET2' => [
                      [ '2','111','ERASER'],
                      [ '2','200','PEN'],
                      [ '0','220','BLACKBOARD'],
                      [ '1','300','CHALK'],
                    ]
        };

my $key;
my %hoa;

my @arr;
while ( <DATA> )
{  
   chomp; # try taking out chomp and see how test fails.
   next if (/^\s+/);
   if ( /^SET/ ) {
     my @KEY = split (/[\s:]/,$_);
     $key = $KEY[2];
     #clear array
     @arr = (); # try taking this out and see effect on test.
} elsif (/^\d/) {
       my @INST = split(/,/,$_);
       push @arr, [@INST];
    }
   $hoa{$key} = [ @arr];
}
is_deeply (\%hoa, $expected); #fails, because of an error in the expec
+ted data structure.
$expected->{SET2}->[0]->[1]=110;
is_deeply (\%hoa, $expected); #Works now.


__DATA__
SET: SET1

0,100,BOOK
1,150,PENCIL
====

SET: SET2

2,110,ERASER
2,200,PEN
0,220,BLACKBOARD
1,300,CHALK
====

[Anonymous Monk]

Thanks so much tpyahoo,
Glad that I learnt an alternative to Data::Dumper.
Your advice is very valuable indeed.