Making a menu with copy, create, remove and rename

Question

The rename part does not have any code yet, but the rest does have the code to go along with it.

I keep getting that the variable for copy and remove aren't declared, but I did. What am I doing wrong?

#!/usr/bin/perl

use warnings;
use strict;

sub menu {
  my $args = shift;
  my $title = $args->{title};
  my $choices = $args->{choices};

  while (1) {
    print "--------------------\n";
    print "$title\n";
    print "--------------------\n";
    for (my $i = 1; $i <= scalar(@$choices); $i++) {
      my $itemHeading = $choices->[$i-1][0];
      print "$i.\t $itemHeading\n";
    }
    print "\n?: ";
    my $i = <STDIN>; chomp $i;
    if ($i && $i =~ m/[0-9]+/ && $i <= scalar(@$choices)) {
      &{$choices->[$i-1][1]}();
    } else {
      print "\nInvalid input.\n\n";
    }
  }
}

my $menus = {};
$menus = {
  "1" => {
    "title" => "Menu 1 header",
    "choices" => [
      [ "Create file " , sub {
        print "Name of file you want to create:  ";
        my $createFile = <STDIN>;
        open my $fh, ">>", "$createFile" or die "can't open\n";
        print $fh "Here's that file you ordered\n";
        close $fh;

        print "done\n";
      }],

      [ "Rename a file" , sub {
        print "What file do you want to rename (add       ext): ";
        my $oldName = <STDIN>;
        print "What do you want to rename it: ";
        my $newName = <STDIN>;
      }],

      [ "Copy file" , sub {
        print "What file do you want to copy: "
        my $copyFile = <STDIN>;
        print "What do you want to call this file: :";
        my $newFile = <STDIN>;
        copy "$copyFile" , "$newFile" || die "can't copy";
      }],
      [ "Remove file" , sub {
        print "Which file do you want to remove";
        my removeF = <STDIN>;
        open my $fh;
        unlink "$removeF";
      }],
    },
  };

  menu($menus->{1});

Answer 1

I cleaned up missing semicolons, dollar signs, braces, duplicated lines ... and formatting. I also added the missing use File::Copy. Your code wasn't so far off at all but I can see that you got lost in so many little errors. So here is the most important note:

Enter code a little at a time and make sure that it runs cleanly and works as envisioned. Add small sections of code and test, test, test. Build small, rounded, and crude (basic) components of functionality first, so that you can sensibly test them and your design. Then develop further.

Then each error is either easy to spot or educational.

Here is your code (just two functions), cleaned up so that it works and a little more

use warnings;
use strict;
use feature 'say';

use File::Copy qw(copy move);

sub menu {
    my $args = shift;
    my $title = $args->{title};
    my $choices = $args->{choices};

    while (1) {
        say $title, "\n", '-' x length $title;
        for my $i (1..@$choices) {
            my $itemHeading = $choices->[$i-1][0];
            say "$i.\t$itemHeading";
        }
        print "\n?: ";
        my $i = <STDIN>;
        chomp $i;
        if ($i && $i =~ m/[0-9]+/ && $i <= @$choices) {
            $choices->[$i-1][1]();
        } else {
            say "\nInvalid input.\n";
        }
    }
}

my $menus = {
    1 => {
        "title" => "Menu 1 header",
        "choices" => [
            [ "Create file " ,
                sub {
                    print "Name of file you want to create:  ";
                    my $createFile = <STDIN>;
                    chomp $createFile;
                    open my $fh, ">>", $createFile 
                        or die "Can't open $createFile: $!\n";
                    say $fh "Here's that file you ordered";
                    close $fh;

                    say "done";
                }
            ],
            [ "Rename a file" ,
                sub {
                    print "What file do you want to rename (add ext): ";
                    my $oldName = <STDIN>;
                    chomp $oldName;
                    if (not -e $oldName) {
                        say "\n\tNo file \"$oldName\"\n";
                        return;
                    }
                    print "What do you want to rename it: ";
                    my $newName = <STDIN>;
                    chomp $newName;          # what if it exists already?
                    move $oldName, $newName
                        or die "Can't move $oldName to $newName: $!";
                }
            ],
            [ "Quit", sub { exit } ]
        ]
    }
};

menu($menus->{1});

I also added the option to quit and some error checking.

A few notes

• There is rarely ever a need for a C-style for loop

• The & in front of a sub name has a very specific purpose. Normally it is not needed

• Add $! when you print error to actually see what it is, and/or use other error variables

• When an array is in scalar context its length is used, the number of elements; so there is no need for scalar in your conditions, just 1..@$choices and $i <= @$choices

• Don't quote needlessly, it can lead to an error. Quoting is needed mostly when variables are interpolated in a string. No need for quotes in say $var; or copy $file, $new ...

• The one direct error was missing chomps. With a resource that uses newlines (file, STDIN) that should be a basic habit. Omitting it in some situations may give cleaner flow but openness to that may invite debugging sessions, of "strange" errors possibly far down in code.

Please do your best to present a readable question with properly formatted code.

Answer 2

You have an extremely detailed answer from zdim. That's the one you should accept. But here's a list of what I had to fix in order to get your code running:

• Removed the duplicate print "done\n" and the following close brackets at the end of the first menu option subroutine (I removed them from your question while reformatting your code).
• Added the missing use File::Copy.
• Added a missing semicolon at the end of the first print statement in the copy option.
• Added a missing $ to the my removeF statement in the remove option.

All of these were found by running your code through perl -c (which is Perl's built-in syntax checker).

In addition, you should take notice of all of zdim's suggested improvements.

And please format your code more carefully.