Search code examples
perlrecursioncode-organization

Simplifying recursive function


While wrapping my brains around recursion, I hope to with the below simple problem to better understand recursion itself, but also in the spirit of TIMTOWTDI to other or better ways to achieve my goal.

Problem is pretty simple, given an complex Perl data structure, $cds (that guarantees to start with HASH, hmm, does it make a diff ?), I wish to traverse $cds and remove single-level-arrays.

I started with the below, which worked fine...

sub reduce_1level
{
    my $cds = shift;

    if (ref $cds eq 'HASH')
    {
        foreach my $key (keys %$cds)
        {
            if (ref $cds->{$key} eq 'ARRAY')
            {
                if (@{$cds->{$key}} == 1)
                {
                    $cds->{$key} = $cds->{$key}[0];
                }
                else
                {
                    reduce_1level ($cds->{$key});
                }
            }
            elsif (ref $cds->{$key} eq 'HASH')
            {
                reduce_1level ($cds->{$key});
            }
        }
    }
    elsif (ref $cds eq 'ARRAY')
    {
        foreach (@$cds)
        {
            reduce_1level ($_) if ref;
        }
    }
}

... but I somehow felt the code could be simpler and shorter. After needling the stack on similar problems, I revised to:

sub reduce_1level
{
    my $cds = shift;

    if (ref $cds eq 'ARRAY')
    {
        foreach (@$cds)
        {
            reduce_1level ($_) if ref;
        }
    }
    elsif (ref $cds eq 'HASH')
    {
        foreach my $key (keys %$cds)
        {
            if (ref $cds->{$key} eq 'ARRAY' and @{$cds->{$key}} == 1)
            {
                $cds->{$key} = $cds->{$key}[0];
            }
            next unless ref $cds->{$key};
            reduce_1level ($cds->{$key});
        }
    }
}

I'm wondering what else or better way can be done to achieve indented goal?


Solution

  • Forget complexity. It's not even correct.

    Some calls handles two levels of the data structure, so you should have duplicated code. But you don't. As a result, not all the single-element arrays in the following are eliminated:

    { foo => [ [ 3 ] ] }
    

    Don't try to handle two levels in the same call.

    sub reduce_1level {
        our $cds; local *cds = \shift;   # alias $cds = shift;
    
        my $reftype = ref($cds)
            or return;
    
        if ($reftype eq 'HASH') {
            reduce_1level($_) for values %$cds;
        }
        elsif ($reftype eq 'ARRAY') {
            if (@$cds == 1) {
                $cds = $cds->[0];
                reduce_1level($cds);
            } else {
                reduce_1level($_) for @$cds;
            }
        }
        else {
            die("Unsupported reference type $reftype\n");
        }
    }