CODEWITHSUNDEEP
c++if-statementboolean-logic
whatsherface
whatsherface

Reputation: 413

C++ boolean logic error possibly caused by if statements

Here is an extremely simplified version of a section of code that I am having trouble with.

int i = 0;
int count = 0;
int time = 50;
int steps = 1000;
double Tol = 0.1;
bool crossRes = false;
bool doNext = true;

for (int i=0; i<steps; i++) {

//a lot of operations are done here, I will leave out the details, the only
//important things are that "dif" is calculated each time and doNext either
//stays true or is switched to false

    if (doNext = true) {
        if (dif <= Tol) count++;
        if (count >= time) {
            i = steps+1;
            crossRes = true;
        }
    }
}

    if (crossRes = true) {
        printf("Nothing in this loop should happen if dif is always > Tol 
               because count should never increment in that case, right?");
    }

My issue is that every time it gets done with the for loop, it executes the statements inside the "if (crossRes = true)" brackets even if count is never incremented.

Upvotes: 0

Views: 934

Answers (5)

Luchian Grigore
Luchian Grigore

Reputation: 258548

I stand corrected:

if (crossRes)
You wouldn't have this problem if your condition was if (true = crossRes) because it wouldn't compile. `crossRes = true` always evaluates to `true` because it's an assignment, to `true`. You want `crossRes == true`: if (crossRes == true) { printf("Nothing in this loop should happen if dif is always > Tol because count should never increment in that case, right?"); }

Upvotes: 2

Jerry Coffin
Jerry Coffin

Reputation: 490048

First, although a number of people have pointed to the problem with if (crossRes = true), for some reason they haven't (yet, anyway) pointed to the same problem with if (doNext = true).

I'll stick to pointing out that you really want if (crossRes) rather than if (crossRes == true) (or even if (true == crossRes)).

The first reason is that it avoids running into the same problem from a simple typo.

The second is that the result of the comparison is a bool -- so if if (crossRes==true) is necessary, you probably need if (((((crossRes == true) == true) == true) == true) just to be sure (maybe a few more -- you never know). This would, of course, be utterly silly -- you're starting with a bool, so you don't need a comparison to get a bool.

I'd also note for the record, that if you insist on doing a comparison at all, you should almost always use if (x != false) rather than if (x == true). Though it doesn't really apply in C++, in old C that doesn't have an actual Boolean type, any integer type can be used -- but in this case, a comparison to true can give incorrect results. At least normally, false will be 0 and true will be 1 -- but when tested, any non-zero value will count as equivalent to true. For example:

int x = 10;

if (x)    // taken

if (x == true) // not taken, but should be.

If you're not starting with a Boolean value as you are here, then the if (<constant> <comparison> <variable>) makes sense and is (IMO) preferred. But when you're starting with a Boolean value anyway, just use it; don't do a comparison to produce another of the same.

Upvotes: 1

mathematician1975
mathematician1975

Reputation: 21351

The other answers here have told you the problem. Often your compiler will warn you but a way to ensure that you do not do this is to put the constant term on the left

 true == crossRes

that way you get a compiler error instead of a warning and so it can't escape unnoticed since

 true = crossRes

wont compile.

Upvotes: 1

Blender
Blender

Reputation: 298056

You've made a common (and quite frustrating) mistake:

if (crossRes = true) {

This line assigns crossRes to true and returns true. You're looking to compare crossRes with true, which means you need another equals sign:

if (crossRes == true) {

Or more concisely:

if (crossRes) {

Upvotes: 5

cdhowie
cdhowie

Reputation: 168958

= is assignment, == is equality comparison. You want:

if (crossRes == true) {

You make the same mistake here:

if (doNext = true) { // Bad code

Upvotes: 1

Related Questions