CODEWITHSUNDEEP
javabooleanoperator-precedence
Joel
Joel

Reputation: 2721

simple boolean question

What am I doing wrong here?

I am wanting to display integers from 1-100 who are divisible by either 6 or 7. That's done and working. The next step is to not display any that are divisible by both...that isn't working in my loop (those integers are still displaying)

for (int i = 1; i < 100; i++)
    if (i % 6 == 0 || i % 7 == 0 && i % (6 * 7) != 0){
        println(i);
    }

Thanks! Joel

Upvotes: 2

Views: 393

Answers (5)

paxdiablo
paxdiablo

Reputation: 881503

I would simply stop worrying about how to evaluate precedence, and use something like:

for (int i = 1; i <= 100; i++) {
    if ((i % 42) == 0) continue;
    if ((i %  6) == 0) println (i);
    if ((i %  7) == 0) println (i);
}

I'm assuming here that 1-100 was an inclusive range in which case you should use <= rather than <. It won't matter for your specific case since 100 is divisible by neither 6 nor 7.

Guess what? Any decent optimising compiler (including JIT ones) will probably end up generating the same code as for all the other possibilities. And, even if it didn't, it wouldn't matter unless you were calling this function a great many times.

I think that's a tiny bit more readable than:

if (i % 6 == 0 || i % 7 == 0 && i % (6 * 7) != 0) ...

or, worse yet, the Lisp-like thing you'll have to turn it into to get it working properly :-)

Keep in mind one possibility - you can change your loop to make it more efficient (sevenfold), for the specific case with 6 and 7, thus:

for (int i = 7; i <= 100; i += 7)
    if ((i % 6) != 0)
        println (i);

This uses the for loop itself to only check multiples of 7 and print them if they're not also multiples of 6.

Upvotes: 2

objects
objects

Reputation: 8677

for (int i = 1; i < 100; i++)
if ((i % 6 == 0 || i % 7 == 0) && !(i % 6 == 0 && i % 7 == 0)){
    println(i);
}

Upvotes: 1

user85421
user85421

Reputation: 29680

You could also use the exclusive or 

    for (int i = 1; i < 100; i++) {
        if ((i % 6 == 0) ^ (i % 7 == 0)) {
            println(i);
        }
    }

or just an unequal if ((i % 6 == 0) != (i % 7 == 0))
Since exclusive or is not used that often, I doubt this will increase the readability of the code...

Upvotes: 2

Mark Byers
Mark Byers

Reputation: 838226

Missing parentheses:

for (int i = 1; i < 100; i++) {
    if ((i % 6 == 0 || i % 7 == 0) && i % (6 * 7) != 0) {
        println(i);
    }
}

Upvotes: 3

radai
radai

Reputation: 24192

try making your condition more explicit by adding (...), like so:


if (((i % 6 == 0 || i % 7 == 0) && (i % (6 * 7) != 0)) {
}

by default && takes precedence over ||

Upvotes: 5

Related Questions