Switch statement returning wrong

Question

I started to learn Java some time ago and I thought that making a calculator that works in the terminal. Lately I added an Array list to store the history, and then something went wrong. Calculator program:

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

public class calc_case {
    public static void main(String[] args) {
        System.out.println("Welcom to The Calculator!");
        double a;
        double b;
        double c;
        Scanner input0;
        int input = 0;
        ArrayList<Double> history = new ArrayList<Double>();
        while (input != 6) {
            try {Thread.sleep(2000);} catch(InterruptedException ex) {Thread.currentThread().interrupt();}
            a = 0; b = 0; c = 0; input = 0;
            System.out.println("#################################################");
            System.out.println("How can I help you?");
            System.out.println("1-Add\n2-Subtrackt\n3-Devide\n4-Multiply\n5-Show history\n6-Exit");
            input0 = new Scanner(System.in);
            input = input0.nextInt();
            switch (input) {            
                case 1: //add
                    System.out.println("Input two numbers:");
                    input0 = new Scanner(System.in);
                    a = input0.nextDouble();
                    input0 = new Scanner(System.in);
                    b = input0.nextDouble();
                    c = a + b;
                    System.out.println("Calculating... \nThe answer is: " + c );
                    break;
                case 2: //subtrackt
                    System.out.println("Input two numbers:");
                    input0 = new Scanner(System.in);
                    a = input0.nextDouble();
                    input0 = new Scanner(System.in);
                    b = input0.nextDouble();
                    c = a - b;
                    System.out.println("Calculating... \nThe answer is: " + c );
                    break;
                case 3: //devide
                    System.out.println("Input two numbers:");
                    input0 = new Scanner(System.in);
                    a = input0.nextDouble();
                    input0 = new Scanner(System.in);
                    b = input0.nextDouble();
                    c = a/b;
                    System.out.println("Calculating... \nThe answer is: " + c );
                    break;
                case 4: //multiply
                    System.out.println("Input two numbers:");
                    input0 = new Scanner(System.in);
                    a = input0.nextDouble();
                    input0 = new Scanner(System.in);
                    b = input0.nextDouble();
                    c = a*b;
                    System.out.println("Calculating... \nThe answer is: " + c );
                    break;
                case 5: //history
                    for (int x = 0; x < history.size(); x++) {
                        System.out.println(history.get(x));
                    }
                case 6: //exit
                    System.out.println("Goodbye!\n Killing process... " + " OK");
                default:
                    history.add(c);
            }
        }
    }
}

After adding case 5 as 'Show history' and moving case 5 'Exit' to case 6 'Exit' after choosing option 5 ('Show history') I get this:

Goodbye!
 Killing process... OK

I tried recompiling the program, deleting the class file, and pasting the program to a new file. What is wrong with my program?

OS: Ubuntu 16.04 Java version:
Java version: openjdk version "1.8.0_91"

There is no `break` for case 5 or 6. – Vince Aug 09 '16 at 19:02 — Vince, Aug 09 '16 at 19:02

GhostCat · Accepted Answer · 2016-08-09T19:10:03.537

Two things:

You forget the break after case 5; so you always fall through to the next case. So does the next case 6.
Your history is printed; but you never add a value to that history! There is not much sense in keeping a history, if that variable is only read from, but never written to.

And from a "clean coding" perspective: read about the "single layer of abstraction" principle. You really do not want to have so much code in one method; for example: each of your case blocks ... should go into its own method. You want to create small "units" that can be understood with one glance. Something that goes over 10 lines or more always requires more work than something smaller!

And you know; when each code would like this: ...

case 3: 
  divide();
  break;
case 4:
  multiply();
  break;

dont you think you would have spotted your problem yourself?!

Thank you for not just answering the question, but for providing feed back to help the asker learn better methods of coding. — Susannah Potts, Aug 09 '16 at 19:10

score 2 · Answer 2 · answered Aug 09 '16 at 19:09

2

You do not have break in case 5, that is why it executes case 6 after finishing case 5

answered Aug 09 '16 at 19:09

user2044822

135
6

score 1 · Answer 3 · answered Aug 09 '16 at 19:04

1

You are missing a break after your for loop in case 5:, so it falls through into case 6:.

Also, case 6: appears to be missing a break after the System.out statement.

answered Aug 09 '16 at 19:04

Bdoserror

1,184
9
22

Switch statement returning wrong

3 Answers3