Console application menu

Sometimes I need to write a simple Java console applications and menu look:

do {
    System.out.println("please select menu item");
    System.out.println("1-create, 2-remove, 3 - rename");
    int select;
    Scanner scanner = new Scanner(System.in);
    select = scanner.nextInt();

    switch (select) {
        case 1:
            System.out.println("please select menu item");
            System.out.println("1-image, 2-txt");
            select = scanner.nextInt();
            if (select == 1) fileHolder.create("image");
            else if (select == 2) fileHolder.create("image");
            else System.out.println("invalid menu item");
            break;
        case 2:
            System.out.println("please select menu item");
            System.out.println("1-file from pc1, 2-file from pc2, 3-file from pc3");
            select = scanner.nextInt();
            if (select == 1) fileHolder.removeFile("pc1");
            else if (select == 2) fileHolder.removeFile("pc2");
            else if (select == 3) fileHolder.removeFile("pc3");
            else System.out.println("invalid menu item");
            break;
        case 3:
            System.out.println("please type new name");
            String newName = scanner.next();
            fileHolder.rename(newName);
            break;
    }

} while (select != 4);

But I believe that it is not the best choice. I believe that I should use any pattern to improve my code. By description - the state pattern is applicable for my situation but I have no idea how I can use this in this situation. The menu should show the respective menu according to the internal state.

First of all, there are at least three buggy things:

-
select variable is declared inside the do... while loop, so the condition in while is checked against another one (probably, a field in the class?). So, the loop won't work as expected!

-
copy-paste is evil. The calls fileHolder.create("image"); are repeated in two different conditionals. The "txt" option is not used. Please pay more attention to the things you write and do not copy-paste such elementary things.

-
the Scanner is not closed after its usage, which is desirable, as for any other items that use I/O.

For the rest, the code seems quite straightforward, rigid and repetitive, hence difficult to maintain. It contains many if - else if clauses that smell bad.

Of course, the whole story may be refactored with patterns usage. But I don't think that the state pattern suits well here. It'd be better refactored with the command pattern.

All the menu actions (create, remove, rename and their dependent ones) may be wrapped into separate objects, which associate the expected keys (1,2,3..), the text and other necessary stuff and sugar.

For example, a (very short) version of Menu class could be like this:

class Menu {

  private final String action;
  private final Consumer consumer;

  Menu(String action, Consumer consumer) {
    this.action = action;
    this.consumer = consumer;
  }

  void execute() {
    this.consumer.accept(action);
  }
}

And it would be initialized and used:

Menu create = new Menu("create", (a) -> fileHolder.create(a));
// to trigger the execution:
create.execute();

But for this very concrete and short example of code I would consider decomposing it into patterns as a sort of overkill.

There is some job to do about the original straightforward approach, so let's just try to improve it.

The input numbers are read several times using the Scanner object. They should also be validated, so a dedicated method would be very useful:

// int minValue may also be defined if necessary
private static int askUserForNumberInput(Scanner scanner, String prompt, int maxValue) {
  System.out.println("please select menu item");
  System.out.println(prompt);
  int value = scanner.nextInt();
  while (value  maxValue) {
    System.out.println("invalid menu item, please try again");
    // java.util.InputMismatchException should also be caught 
    // to intercept non-numeric input
    value = scanner.nextInt();
  }
  return value;
}

The frame for the main logic would look like this:

try (Scanner scanner = new Scanner(System.in)) {
  final int mainMenuSelection = askUserForNumberInput(scanner, "1-create, 2-remove, 3 - rename", 3);
  switch (mainMenuSelection) {
    // cases
  }
}

And there is not a single if - if else in the cases any more:

case 1: {
  String[] createActions = { "image", "txt" }; // to be extracted to constants
  int createOption = askUserForNumberInput(scanner, "1-image, 2-txt", 2);
  fileHolder.create(createActions[createOption - 1]);
  break;
}
case 2: {
  int removeIndex = askUserForNumberInput(scanner, "1-file from pc1, 2-file from pc2, 3-file from pc3", 3);
  fileHolder.removeFile("pc" + removeIndex);
  break;
}
case 3: {
  System.out.println("please type new name");
  String newName = scanner.next();
  fileHolder.rename(newName);
  break;
}

StackExchange Code Review Q#106401, answer score: 3