For starters, you're using parentheses instead of curlies to create your inner hashes (within the SUBMENU keys). This means the data structure you think you have isn't the data structure you actually created. Try passing %menus through Data::Dumper (this is a great module that I find is invaluable for debugging).

Secondly, the print_main_menu and print_sub_menu functions are very similar. I'd suggest refactoring them into a single, more generalized sub that takes a hash reference as a parameter. Tweaking your code slightly gives this:

sub print_menu 
{
    my ( $href ) = @_;
    print "$_\t", $href->{$_}, "\n"
        for ( sort keys %{ $href } );
}
[download]

There are additional errors, also. For example, I don't think you really wanted $menu->{$_} (in print_main_menu), unless you have a hash ref assigned to $menu somewhere that isn't shown in this example. I suspect you meant to refer to %menus, which is not a reference so the keys would be accessed by $menus{$_}.

Finally, since it wasn't obvious in your example, use strict and warnings! :-)

Thanks - I glazed over the () when looking at the code, that did the trick.


Ted
--
"That which we persist in doing becomes easier, not that the task itself has become easier, but that our ability to perform it has improved."
  --Ralph Waldo Emerson

I think what you're trying to achieve here (correct me if I'm wrong) is to display a submenu only when it's needed, i.e. if the user has selected an item from the main menu which has a submenu. To do this, you're wanting to defer the printing of a submenu. Your code, as shown, does not acheive this; even though you're taking a reference to the print_sub_menu function, you're still calling it right away, and putting the result in the data structure. Instead, you should put a code reference in the data structure. E.g.:

my %menus = (
  '1' => {
    'ITEM'    => 'List and Kill UDT* processes by user',
    'ACTION'  => sub { $RunCMD->('ListKillProc') },
  },

  '2' => {
    'ITEM'    => 'List and Kill Print Jobs',
    'ACTION'  => sub { $PrintSubmenu->('2') },
    . . .
[download]

Then, when the user selects an item, you can execute the action; something like this:

    $menu->{$MenuItem}->{'ACTION'}->();
[download]

Other than that, there's a lot you could do to this to make it more generic and robust.

Might I also suggest that you're suffering from (or inflicting) Indentation Hell. Try it this way:

my %menus = (
    '1' => {
        'ITEM'    => 'List and Kill UDT* processes by user',
        'ACTION'  => $RunCMD->('ListKillProc'),
    },

    '2' => {
        'ITEM'    => 'List and Kill Print Jobs',
        'ACTION'  => $PrintSubmenu->('2'),
        'SUBMENU' => {
            '1' => {
                'ITEM'   => 'Show all Printers',
                'ACTION' => $RunCMD->('showprintersall'),
            },
            '2' => {
                'ITEM'   => 'Show user print jobs',
                'ACTION' => $RunCMD->('showprintersuser'),
            },
            '3' => {
                'ITEM'   => 'Show single printer',
                'ACTION' => $RunCMD->('showprinter'),
            },
            '4' => {
                'ITEM'   => 'Kill a print job',
                'ACTION' => $RunCMD->('killprint'),
            },
        }
    },

    '3' => {
        'ITEM'    => 'Manage user accounts',
        'ACTION'  => $PrintSubmenu->('3'),
        'SUBMENU' => {
            '1' => {
                'ITEM'   => 'Unlock user account',
                'ACTION' => $RunCMD->('unlockuser'),
            },
            '2' => {
                'ITEM'   => 'Change account password',
                'ACTION' => $RunCMD->('changepass'),
            },
        },
    },

    '4' => {
        'ITEM'    => 'Run App',
        'ACTION'  => $RunCMD->('APP'),
    },
                                                         
    '5' => {
        'ITEM'    => 'quit',
        'ACTION'  => $RunCMD->('quit'),
    },
);
[download]

Inspired by the OP, I wrote the following. It's a system for simple console-based menus. It's easily extensible.

use strict;
use warnings;

sub do_menu
{
    my( $menu ) = @_;
    while(1)
    {
        my( $menu ) = @_;
        # display the menu
        print "\n";
        print $_+1, '. ', $menu->[$_]{'label'}, "\n"
            for 0 .. $#{$menu};
        print '0. ', ( @_ > 1 ? 'Return' : 'Exit' ), "\n";
        # get the user's input
        local @ARGV;
        print STDERR '> ';
        local $_ = <>; chomp;

        /\d/ && !/\D/ or next;
        $_ == 0 and last; # item 0 is special
        defined $menu->[$_-1] or warn("Invalid choice\n"), next;

        my $op = $menu->[$_-1]{'op'};
        my $arg = $menu->[$_-1]{'arg'};
        if ( $op eq 'submenu' )
        {
            do_menu( $arg, @_ ); # maintain the stack!
        }
        elsif ( $op eq 'exec_cmd' )
        {
            execute_command( $arg );
        }
        else
        {
            warn "Unrecognized op '$op'\n";
        }
    }
}


my @printers_menu = (
    {
        label => 'Show all Printers',
        op => 'exec_cmd',
        arg => 'showprintersall',
    },
    {
        label => 'Show user print jobs',
        op => 'exec_cmd',
        arg => 'showprintersuser',
    },
    {
        label => 'Show single printer',
        op => 'exec_cmd',
        arg => 'showprinter',
    },
    {
        label => 'Kill a print job',
        op => 'exec_cmd',
        arg => 'killprint',
    },
);

my @accounts_menu = (
    {
        label => 'Unlock user account',
        op => 'exec_cmd',
        arg => 'unlockuser',
    },
    {
        label => 'Change account password',
        op => 'exec_cmd',
        arg => 'changepass',
    },
);

my @main_menu = (
    {
        label => 'List and Kill UDT* processes by user',
        op => 'exec_cmd',
        arg => 'ListKillProc',
    },
    {
        label => 'List and Kill Print Jobs...',
        op => 'submenu',
        arg => \@printers_menu,
    },
    {
        label => 'Manage user accounts...',
        op => 'submenu',
        arg => \@accounts_menu,
    },
    {
        label => 'Run App',
        op => 'exec_cmd',
        arg => 'APP',
    },
);

sub execute_command
{
    warn "Executing command '@_'\n";
}

do_menu( \@main_menu );
[download]

Of course, with objects, this might have been even more elegant.

We're building the house of the future together.

Another thing not necessarily related to your curly/paren confuzzling is the lines like ACTION => $RunCMD->('ListKillProc'). Unless these are returning coderefs or objects which actually implement the behavior in question you're running everything all at once when you define %menus. You might be after something more along the lines of ACTION => sub { $RunCMD->('ListKillProc') }.

Then again you may be doing the right thing and these are instances or coderefs already, in which case . . . never mind.</Emily Litella>