Loop not working properly

Question

EDIT: I changed my previous code where I compared strings with !=, to .equals(), but it's still the same.

I'm trying to make a beginner calculator. I'm stuck in this infinite loop which I can't figure out.

First I tried this:

public void GetValues()
{
    System.out.println("Welcome to the \"Math Calculator\"\n");

    System.out.print("Type in the first number: ");
    int FirstNumber = Scan.nextInt();

    System.out.print("Type in the second number: ");
    int SecondNumber = Scan.nextInt();

    System.out.println("\nWhat operation would you like to do?\n");
    System.out.println("For addition, type \"+\"");
    System.out.println("For subtraction, type \"-\"");
    System.out.println("For multiplication, type \"*\"");
    System.out.println("For division, type \"/\"\n");

    MathOperation = Scan.next();

    while (MathOperation != "+" || MathOperation != "-" || MathOperation != "*" || MathOperation != "/")
     {
        System.out.print("The value you typed, is not valid. Type again: ");
        MathOperation = Scan.next();
     }

}

Still, no matter what I type in, I still get this message The value you typed, is not valid. Type again:

I just can't figure it out. What am I missing?

EDIT2: I changed the loop into this:

    while (!IsEqual)
    {
        System.out.print("The value you typed, is not valid. Type again: ");
        MathOperator = Scan.next();

        if (MathOperator.equals("+") || MathOperator.equals("-") || MathOperator.equals("*") || MathOperator.equals("/"))
        {
            IsEqual = true;
        }
    }

And now it works. Thanks to all who put effort on helping me. Cheers :)

Compare strings with `equal()` not with `==` so modify condition like `!MathOperation.equal("+")` — Suresh Atta, Oct 18 '13 at 11:42
[why-doesnt-work-on-string](http://stackoverflow.com/questions/17443201/why-doesnt-work-on-string/17443215#17443215) — Suresh Atta, Oct 18 '13 at 11:44
`I changed my previous code where I compared strings with !=, to .equals()` then post this code instead of old one. — Pshemo, Oct 18 '13 at 11:50
Best practice, code as "+".equals(MathOperation) because MathOperation may be null... — Hariharan, Oct 18 '13 at 11:51

score 1 · Answer 1 · answered Oct 18 '13 at 11:45

1

change this

if (MathOperation == "+" || MathOperation == "-" || MathOperation == "*" || MathOperation == "/")

to

        if (MathOperation.equals("+") || MathOperation..equals("-") || MathOperation.equals("*") || MathOperation.equals("/"))

answered Oct 18 '13 at 11:45

SpringLearner

13,738
20
78
116

Best practice, code as "+".equals(MathOperation) because MathOperation may be null. – Hariharan Oct 18 '13 at 11:50

John · Answer 2 · 2013-10-18T11:53:02.700

1

You should get what you're getting with the second try, even if you had written the comparison correctly using equals().

In essence what you have is

if (it's not all four math operations at the same time)
{
    "invalid"
}

The if clause is always true.

edited Oct 18 '13 at 11:53

answered Oct 18 '13 at 11:47

John

15,990
10
70
110

score 0 · Answer 3 · answered Oct 18 '13 at 11:48

You use invalid method for compare.

The String is not a primitive type it a Object. This mean to compare it with other you must invoke method equlas from it against the validated element.

"+".equals(mathOperation)

Another method to solve this issue is to move from Object to primitive char structure.

char mathOperation = Scan.nextChar(); //Note that variables should start with small letter.

then you can use == and != operators to fetch the result.

if (mathOperation != '+')

score 0 · Answer 4 · answered Oct 18 '13 at 12:05

Of course you should always ues .equals method for every object comparison ( not only strings ) Performing == operation on object checks whether they are pointing at the same memory space which is different than checking whether two values are the same.

If you also hate conditional logic as I do you can go for something like this:

Vector<String> allowedMathOperators = new Vector<String>();
allowedMathOperators.add("+");
allowedMathOperators.add("-");
allowedMathOperators.add("*");
allowedMathOperators.add("/");

String mathOperation = "";
Scanner scan = new Scanner(System.in);

do{
    System.out.println("What operation would you like to do?");
    mathOperation = scan.next();
}while( !allowedMathOperators.contains( mathOperation ));

The advantage of my solution is that if you decide to add support for additional operator then you don't have to extend your conditional logic you just add char to allowedMathOperators :)

Loop not working properly

4 Answers4