3
\$\begingroup\$

I wrote this demo just for practice purpose, I would like to get some helps that could further improve my code quality. Any suggestion for improvement would be appreciated.

This code is a simple demonstration of stack implementation (by using array List).

  1. User will be asked for the option of a function to demonstrate.
  2. Once user made the choice (either use stack algorithm or java Stack), the corresponding function will ask user for size of the stack and elements that user wants to put into the stack.
  3. Program will ask for the basic stack operation(push & pop).
  4. Program will receive user inputs(integer) and push into the stack then print current stack.
  5. After the print is completed, program will continue asking for new input, until user want to clear the stack and exit the function.

Note:

  1. If the stack is overflowed, user will be notified and program will add additional space to the stack(perk for using Linked list).
  2. If stack is empty, user will be notified and program will break out of the pop option until user deiced to push in some elements.

    import java.util.ArrayList;
    import java.util.InputMismatchException;
    import java.util.Scanner;
    
    public class basicStack {
    
    public static void main(String[] args) {
        System.out.println("Welcome to basic Stack test, please choose your testing subject: ");
        try(Scanner user_option = new Scanner(System.in)){
    
        mainMenu(user_option);
    
        }//close the I/O before the program end
        System.out.println("Program terminated...");
    
    }
    
    private static void mainMenu(Scanner input){
        int option = 0;
    
        do{
    
            System.out.println("Option 1: Simple stack algorithm implementation");
            System.out.println("Option 2: Simple java library stack usage");
            System.out.println("Option 0: Exit program");
    
            option = input.nextInt();
            System.out.println("Current option is: " + option);
    
            switch(option){
    
            case 1:
                System.out.println("running 1");
                constructStackAlg();
                break;
            case 2:
                System.out.println("running 2");
                //constructJavaStack();
                break;
            case 0:
                return;
            default:
                System.out.println("Invaild Option, Please choose a vaild option...");
                break;
            }
    
            System.out.println("-Main Menu-");
    
        }while(option != 0);
    }
    
    private static void constructStackAlg(){
        int optionS1 = 1;
        Scanner sc_Stack1 = new Scanner(System.in);
            do{
                int size = getS1Size();
                ArrayList<Integer> s1 = buildStack(size);
                stackFunction(s1, size);
                //printS1(s1);
                System.out.println("Stack cleared...");
                System.out.println("Do you wish to test another Stack? "
                                    + "Press any button to contiune or "
                                    + "enter 0 to exit Stack test.");
                optionS1 = sc_Stack1.nextInt();
            }while(optionS1 != 0);
        System.out.println("\nEnd of testing 1, back to main menu...");
    }
    
    private static int getS1Size(){
        int s1Size = 0;
        Scanner scGetS1Size = new Scanner(System.in);
    
        try{
            System.out.println("Please enter the size of this Stack:");
            s1Size = scGetS1Size.nextInt();
        }catch(InputMismatchException exception){
            System.out.println("Please input a vaild integer.");
        }
    
        return s1Size;
    
    }
    
    private static ArrayList<Integer> buildStack(int s1Size){
        ArrayList<Integer> resultStack;
        resultStack = new ArrayList<Integer>(s1Size);
        return resultStack;
    }
    
    private static void stackFunction(ArrayList<Integer> s1, int s1Size){
        ArrayList<Integer> stack1 = s1;
        int stack1Size = s1Size;
        System.out.println("Please choose the stack option:");
        Scanner scStackFunction = new Scanner(System.in);
        Scanner scElement = new Scanner(System.in);
        int option = 3;
        int element = 0;
        do{
            System.out.println("1.Push \t 2. Pop  3. Clear stack and exit current test");
            System.out.println("Your option: ");
            option = scStackFunction.nextInt();
            switch(option){
                case 1:
                    System.out.println("Please input the integer you would like to store: ");
                    element = scElement.nextInt();
                    checkOverflow(stack1,stack1Size);
                    push(stack1, element);
                    printS1(stack1);
                    break;
                case 2:
                    if(checkEmptyStack(stack1)){
                        System.out.println("Stack is empty.");
                        break;
                    }
                    pop(stack1);
                    printS1(stack1);
                    break;
                case 3:
                    return;
                default: 
                    System.out.println("Please enter a vaild option.");
                    break;
            }
        }while(option != 3);
    
    
    }
    
    private static void push(ArrayList<Integer> stack1, int element1){
        stack1.add(element1);
    }
    private static int pop(ArrayList<Integer> stack1){
        int element;
        element = stack1.remove((stack1.size()-1));
        return element;
    }
    private static void printS1(ArrayList<Integer> stack1){
        System.out.println("Now Stack: " + stack1);
    }
    private static void checkOverflow(ArrayList<Integer> stack1, int s1Size)
    {
        if(stack1.size() > s1Size - 1)
            System.out.println("Stack overflow, adding additional space...");
    }
    private static boolean checkEmptyStack(ArrayList<Integer> stack1)
    {
        return stack1.isEmpty();
    }
    }
    
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

The try-with-resources in main

try(Scanner user_option = new Scanner(System.in)){
    mainMenu(user_option);
}//close the I/O before the program end

is a nice idea, as is passing the Scanner into the rest of the program but later in the program we create several new Scanner instances

private static void constructStackAlg(){
    int optionS1 = 1;
    Scanner sc_Stack1 = new Scanner(System.in);
    //...

private static int getS1Size(){
    int s1Size = 0;
    Scanner scGetS1Size = new Scanner(System.in);
    //...

private static void stackFunction(ArrayList<Integer> s1, int s1Size){
    ArrayList<Integer> stack1 = s1;
    int stack1Size = s1Size;
    System.out.println("Please choose the stack option:");
    Scanner scStackFunction = new Scanner(System.in);
    Scanner scElement = new Scanner(System.in);
    //...

none of which are contained in a try-with-resources. A consistent approach to such things is good - if it is worth using it on one of the Scanners it should be used on all. If it is not needed for the later Scanners, why use it on the first. (note: It should be used whenever as it prevents resource leakage). Also, we create two Scanners in stackFunction is there a reason for this? It seems to work with only one scanner used for both the function selection and the element entry.

As an alternative, instead of creating each of the new Scanners we could try passing the Scanner created in main through to each of the other routines.

The post is tagged as Beginner so I do not know if you are comfortable with creating classes, but if you are, a third alternative is to create a class for the Demo, passing in the Scanner, storing it as a member variable and using it in each routine.

Structure
In mainMenu the

while(option!=0) 

will never be false because

case 0:
    return;

will return you from the method. As an alternative you can use

while(true);

which will give you an infinite loop terminated by the return on case 0

or

case 0:
    break;

which will do nothing on case 0 but when you get to the while will exit the loop.

Error Handling
The only place we validate the input (check for an exception) is in getS1Size but in every place where we use .nextInt() there is a possibility of an InputMismatchException. If, at the mainMenu, the user enters 'a', the program crashes with the above exception.

As with using try-with-resources on the scanners, if it is worth checking for the exception when getting the stack size, it is worth checking for it wherever we get input. Also, we do not really handle the error here. We ask the user to enter a valid integer but do not give them the opportunity to do so and continue on to the next stage with a stack size of 0.

Note:If we share a scanner between the routines we can end up with an infinite loop if you have an exception, this answer gives a way to deal with this. One of the other answers does mention creating a new Scanner each time we want to read input as a way of bypassing this problem but some of the point of the Scanner is that is provides access to a Stream. In this case the stream is Console.in and creating a new scanner each time is possible. Say it was a file, or a string or any stream where we cared about the position in the stream.

EDIT

We can encapsulate the demo in a class allowing us to refactor an amount of the common functionality

import java.util.ArrayList;
import java.util.InputMismatchException;
import java.util.Scanner;

public class BasicStack {

    public static void main(String[] args) {

        System.out.println("Welcome to basic Stack test, please choose your testing subject: ");

        try(Scanner scanner = new Scanner(System.in)){
            new StackExample().run(scanner);
        }//close the I/O before the program end

        System.out.println("Program terminated...");

    }
}

public class StackExample {

    private Scanner _scanner;

    public void run(Scanner scanner){
        _scanner = scanner;
        mainMenu();
    }

    private void mainMenu(){

    do{

        writeLine("Option 1: Simple stack algorithm implementation");
        writeLine("Option 2: Simple java library stack usage");
        writeLine("Option 0: Exit program");

        int option = readNumber();
        writeLine("Current option is: " + option);

        switch(option){

        case 1:
            writeLine("running 1");
            constructStackAlg();
            break;
        case 2:
            System.out.println("running 2");
            //constructJavaStack();
            break;
        case 0:
            return;
        default:
            writeLine("Invaild Option, Please choose a vaild option...");
            break;
        }

        writeLine("-Main Menu-");

    }while(true);
}

private void constructStackAlg(){
    int optionS1 = 1;

        do{

            int size = 0;
            while(size <1){
                writeLine("Please enter the size of this Stack:");
                size = readNumber();
                if(size<1){
                    writeLine(String.format("%1$d is not a valid size", size));
                }
            }

            ArrayList<Integer> s1 = buildStack(size);
            stackFunction(s1, size);
            //
            writeLine("Stack cleared...");
            writeLine("Do you wish to test another Stack? "
                                + "Press any button to contiune or "
                                + "enter 0 to exit Stack test.");
            optionS1 = readNumber();
        }while(optionS1 != 0);
    writeLine("\nEnd of testing 1, back to main menu...");
}


private ArrayList<Integer> buildStack(int size){
    return new ArrayList<Integer>(size);
}

private void stackFunction(ArrayList<Integer> stack, int s1Size){

    writeLine("Please choose the stack option:");

    do{
        writeLine("1.Push \t 2. Pop  3. Clear stack and exit current test");
        writeLine("Your option: ");
        int option = readNumber();
        switch(option){
            case 1:
                writeLine("Please input the integer you would like to store: ");
                int element = readNumber();
                checkOverflow(stack, s1Size);
                push(stack, element);
                printS1(stack);
                break;
            case 2:
                if(checkEmptyStack(stack)){
                    writeLine("Stack is empty.");
                    break;
                }
                pop(stack);
                printS1(stack);
                break;
            case 3:
                return;
            default: 
                writeLine("Please enter a vaild option.");
                break;
        }
    }while(true);


}

private static void push(ArrayList<Integer> stack1, int element1){
    stack1.add(element1);
}
private static int pop(ArrayList<Integer> stack1){
    int element;
    element = stack1.remove((stack1.size()-1));
    return element;
}

private void printS1(ArrayList<Integer> stack1){
    writeLine("Now Stack: " + stack1);
}

private void checkOverflow(ArrayList<Integer> stack1, int s1Size)
{
    if(stack1.size() > s1Size - 1)
        writeLine("Stack overflow, adding additional space...");
}

private static boolean checkEmptyStack(ArrayList<Integer> stack1)
{
    return stack1.isEmpty();
}

// reads a number or returns -1 if no number available

private int readNumber(){
    int ret = -1;
    try{
        ret = _scanner.nextInt();
    }catch(InputMismatchException exception){
        _scanner.next();
    }

    return ret;

}

protected void writeLine(String line){
    System.out.println(line);
}

}

(Apologies for the indenting, the editor copied tabs across and they are a bit messed up and I do not have time to fix it at the moment)

Important points

  1. We have a single scanner and a common routine for reading numbers that handles the exception and returns a recognisable error (-1) allowing us to prompt for re-entry.

  2. We are routing all the writing through a single point writeLine. I have an abhorrence for writing to System.out in the body of code - it is very limiting and makes unit testing a pain.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for the input. That scanner warning were really bothering me. Could you please explain or give me a reference link about the third alternative method you mentioned? \$\endgroup\$
    – John
    Mar 22, 2017 at 18:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.