patternjavaMinor
Object Oriented Console menu in Java
Viewed 0 times
consolejavamenuorientedobject
Problem
I am a Spanish programming teacher in a vocational training course and I want my students to make a CRUD exercise on arrays.
I want them to start using OOP as well and as an example, I'm going to give them this example of OO Console Menu. We haven't seen Java 8 so far.
Any comments or improvements would be very appreciate.
MenuOption.java
Menu.java
```
package es.primerok.menu;
import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;
/**
* Created by Miguel-David Orrico on 18/02/2017.
*/
public class Menu {
private static final String OPTION_EXIT="0";
private static final String LEVEL_INDENTATION =" ";
private List menuOptions=new ArrayList<>();
private int level=0;
public Menu(){}
public Menu(int level){
this.level=level;
}
public void add(MenuOption m){
this.menuOptions.add(m);
}
public void loopUntilExit(){
String input;
try(Scanner sc=new Scanner(System.in)) {
while (true) {
System.out.println(this);
input = sc.nextLine();
if (input.equals(OPTION_EXIT)) {
return;
}
boolean foundOption = false;
for (MenuOption m : menuOptions) {
if (m.getOption().equalsIgnoreCase(input)) {
m.doAction();
foundOption = true;
}
}
if (!foundOption) {
System.out.println("Sorry, unrecognized option");
}
}
}
}
@Override
public String toString(){
StringBuild
I want them to start using OOP as well and as an example, I'm going to give them this example of OO Console Menu. We haven't seen Java 8 so far.
Any comments or improvements would be very appreciate.
MenuOption.java
package es.primerok.menu;
/**
* Created by Miguel-David Orrico on 18/02/2017.
*/
public abstract class MenuOption {
private String option;
private String menuLine;
public MenuOption(String option, String menuLine) {
this.option = option;
this.menuLine = menuLine;
}
@Override
public String toString() {
return this.option+" - "+this.menuLine;
}
public String getOption() {
return option;
}
public abstract void doAction();
}Menu.java
```
package es.primerok.menu;
import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;
/**
* Created by Miguel-David Orrico on 18/02/2017.
*/
public class Menu {
private static final String OPTION_EXIT="0";
private static final String LEVEL_INDENTATION =" ";
private List menuOptions=new ArrayList<>();
private int level=0;
public Menu(){}
public Menu(int level){
this.level=level;
}
public void add(MenuOption m){
this.menuOptions.add(m);
}
public void loopUntilExit(){
String input;
try(Scanner sc=new Scanner(System.in)) {
while (true) {
System.out.println(this);
input = sc.nextLine();
if (input.equals(OPTION_EXIT)) {
return;
}
boolean foundOption = false;
for (MenuOption m : menuOptions) {
if (m.getOption().equalsIgnoreCase(input)) {
m.doAction();
foundOption = true;
}
}
if (!foundOption) {
System.out.println("Sorry, unrecognized option");
}
}
}
}
@Override
public String toString(){
StringBuild
Solution
Consistent style
As a teacher, try to lead by example. Sometimes you put whitespace around operators (which is the norm), sometimes you don't, sometimes you prefix an opening brace with space, sometimes you don't. Whatever you do, make sure it is consistent (or configure a code format in your IDE and run the formatter over the whole code.)
Non-OO menu decision
In OO you don't just rip information out of an object to base your decision on it. Instead of
add a method to the MenuOption, to let the option decide, whether it is suited for the input
Variable scope
In your loopUntilExit(), you declare the input String outside the loop - why? This should go to the minimal acceptable scope, that is in the line where you call sc.nextLine().
Law of least surprises
I don't like abusing the toString() method to create a multi-line screen menu. This is not what I'd expect of toString(). Thus, I suggest you create another method to do this, and call it explicitly.
One afterthought
In 20 years of programming Java, I have never seen the use of Scanner in real life (TM). Is it really necessary, that every teacher seems to love this object and base their classes on it? Yes, it is simple, straightforward to use, and probably an easy-to-explore way to teach other principles without the input getting in your way, but it does not hold any practical relevance (at least as far I am concerned.)
As a teacher, try to lead by example. Sometimes you put whitespace around operators (which is the norm), sometimes you don't, sometimes you prefix an opening brace with space, sometimes you don't. Whatever you do, make sure it is consistent (or configure a code format in your IDE and run the formatter over the whole code.)
Non-OO menu decision
In OO you don't just rip information out of an object to base your decision on it. Instead of
if (m.getOption().equalsIgnoreCase(input))add a method to the MenuOption, to let the option decide, whether it is suited for the input
if(m.matchesInput(input))Variable scope
In your loopUntilExit(), you declare the input String outside the loop - why? This should go to the minimal acceptable scope, that is in the line where you call sc.nextLine().
Law of least surprises
I don't like abusing the toString() method to create a multi-line screen menu. This is not what I'd expect of toString(). Thus, I suggest you create another method to do this, and call it explicitly.
One afterthought
In 20 years of programming Java, I have never seen the use of Scanner in real life (TM). Is it really necessary, that every teacher seems to love this object and base their classes on it? Yes, it is simple, straightforward to use, and probably an easy-to-explore way to teach other principles without the input getting in your way, but it does not hold any practical relevance (at least as far I am concerned.)
Code Snippets
if (m.getOption().equalsIgnoreCase(input))if(m.matchesInput(input))Context
StackExchange Code Review Q#155769, answer score: 6
Revisions (0)
No revisions yet.