CODEWITHSUNDEEP
java
E Gordon
E Gordon

Reputation: 3

Java do while or .equals logic problems

Ok so I'm still pretty much a nub to Java, I'm doing my best but I just cant figure out why this wont work. I previously worked in c++ but when I changed universities I had to change languages and now I've got some catching up to do, I've got this fairly simple code here, but either the logic in my do while loop is off or I'm not using .equals() properly, though I have googled a lot and tried many different variations of the problem.

do {
    System.out.println("Please enter a valid mode: ");
    mode = input.nextLine();
    mode = mode.toUpperCase();
    // test statement 1 boolean baa = !mode.trim().equals("DEC");
    // test statement 2 System.out.println(baa);
} while ( mode.trim().equalsIgnoreCase("DEC")
    || !"HEX".trim().equals(mode)
    || !"OCT".trim().equals(mode)
    || !"BIN".trim().equals(mode)); //the error is somewhere in here

I have been testing it and it ticks to true if I say input "DEC" but somehow I still can't exit the loop. Sorry if my problem is really silly.

Upvotes: 0

Views: 101

Answers (3)

ursa
ursa

Reputation: 4591

You can clarify your exit condition with the following:

Collection<String> validMode = Arrays.asList("DEC", "BIN", "HEX", "OCT");
do {
    System.out.println("Please enter a valid mode: ");
    mode = input.nextLine();
    mode = mode.trim().toUpperCase();
} while (!validMode.contains(mode));

Edit: for those who likes HashSet wrapping.

  1. Yes, it would be correct to wrap validMode = new HashSet<>(Arrays.asList(...))

But

  1. It is more code to write/read. A bit, but it breaks paradigm "less code - better code".
  2. It is wrong to argue such improvement with algorithms complexity theory, because in the current case we have no variable N, we have constant N = 4.
  3. It is "early optimization", while it was not proved this code is a bottleneck in your system.
  4. There is no actual measurements, that such improvement would lead to better performance, until you test this change.

In practice it is often array-based list for N=4 works better, then the same size hash-set: faster + less memory consumption.

Upvotes: 3

Buddha
Buddha

Reputation: 4476

It will never come out of the loop because of your condition. This line alone will ensure that you will never ever come out of the loop. 

!"HEX".trim().equals(mode)  || !"OCT".trim().equals(mode)

If you have just !"HEX".trim().equals(mode) you can come out by giving HEX but if you have both giving HEX will make !"OCT".trim().equals(mode) it true and giving OCT will make !"HEX".trim().equals(mode) to true.

So, Ideally I would assume that you want to come out if user didn't give both then right operator would be && intead of || in the while condition. As there are multiple conditions, you have to carefully validate if that works.

Assuming you want to be in loop as long as it is DEC but not (HEX or OCT or BIN, or anything else) below code should work. 

do {
    System.out.println("Please enter a valid mode: ");
    mode = input.nextLine();
    mode = mode.toUpperCase();
} while (mode.trim().equalsIgnoreCase("DEC"));

However if your requirement is to ensure that given mode is DEC, Below code will work. This loop will keep asking the user until he enters DEC

do {
    System.out.println("Please enter a valid mode: ");
    mode = input.nextLine();
    mode = mode.toUpperCase();
} while (!"DEC".trim().equals(mode)));

So, my suggestion would be that you analyse your need carefully.

Upvotes: 3

Leo
Leo

Reputation: 6570

It seems that you're thinking in terms of "repeat...until" loop.

In pseudo-code, it would be something like

repeat {
   read x
} until (x==ASC or x==HEX or x==DEC or x==BIN)

Your construction is using a "do...while" loop, which is exactly the opposite of "repeat...until", so you have to use deMorgan to change the condition, like this

do {
   read x
} while (x!=ASC and x!=HEX and x!=DEC and x!=BIN)

Then you can exit the loop when you find one of the values you want.

Of course, it's easier to maintain your condition using contains() as stated by ursa.

Upvotes: 0

Related Questions