Re^2: Recursive function always returns 0

[jpfarmer]

After a lot of help from liverpole, I think I've straightened this out. At the very least, it's giving me the output I was looking for.

Here's the final version for the curious:

sub unpack_row($@); 
sub unpack_row($@){
    my $hash = shift(@_);
    my @row = @_;

    my @stack;
    if(ref($hash) eq 'HASH'){
        my $header = $hash->{'header'};
        while(my ($key, $value) = each(%$hash)){
             push(@stack, unpack_row($value, (@row, $key)));
        }
    }
    else {
        return([@row, $hash]);
    }
    return @stack;
}

my $hash = {};
$hash->{'foo'}->{'bar'}->{'bat'} = 5;
$hash->{'foo'}->{'bar'}->{'bird'} = 2;
$hash->{'goo'}->{'tab'}->{'bird'} = 2;
print Dumper($hash);
print Dumper(unpack_row($hash, ()));

which generates the following output:

$VAR1 = {
          'goo' => {
                     'tab' => {
                                'bird' => 2
                              }
                   },
          'foo' => {
                     'bar' => {
                                'bat' => 5,
                                'bird' => 2
                              }
                   }
        };
$VAR1 = [
          'goo',
          'tab',
          'bird',
          2
        ];
$VAR2 = [
          'foo',
          'bar',
          'bat',
          5
        ];
$VAR3 = [
          'foo',
          'bar',
          'bird',
          2
        ];

[liverpole]

Good job!

I'm still going to recommend that you stop using prototypes.  Not using prototypes is a lesson I learned from my wise fellow monks.  This article is a "must read" on the subject.

You can also simplify your code somewhat.  The following are just suggestions, of course (ultimately you should do what makes the most sense for you), but here are some things to consider ...

Why not pull all your arguments out of @_ in one fell swoop, with my ($hash, @row) = @_?

When I see an if { ... } else { ... } where the first block is fairly long and the second very short, I often like to reverse them, so the short block is first.  Then you can often get rid of the else clause as well.  So instead of:

 if(ref($hash) eq 'HASH'){
        my $header = $hash->{'header'};
        while(my ($key, $value) = each(%$hash)){
             push(@stack, unpack_row($value, (@row, $key)));
        }
    }
    else {
        return([@row, $hash]);
    }

I prefer the more "balanced" looking:

if(ref($hash) ne 'HASH'){
    return([@row, $hash]);
}

my $header = $hash->{'header'};
while(my ($key, $value) = each(%$hash)){
    push(@stack, unpack_row($value, (@row, $key)));
}

Then I noticed you're not using $header, so you can remove that line.

Additionally, you can make the construction of @stack within the loop into a map statement, and then throw away @stack entirely, and just return the result from map.

Here's the entire program (with a few other simplifications):

use strict;
use warnings;
use Data::Dumper;

sub unpack_row {
    my ($hash, @row) = @_;
    return [ @row, $hash ] unless (ref $hash eq 'HASH');
    return map { unpack_row($hash->{$_}, @row, $_) } keys %$hash;
}

my $hash = {
    foo => { bar => { bat => 5, bird => 2 } },
    goo => { tab => { bird => 2 } },
};

print Dumper($hash);
print Dumper(unpack_row($hash, ()));

Isn't that a bit easier to follow?