(c++) for loop is being skipped

Question

The following code prints count as 0. But s.length() is 11 so shouldn't count be 10011?

int main() {
    clock_t start_time = clock();
    string s = "hello world";
    int count = 0;
    for (int i = -10000; i < s.length(); i++) {
        count++;
    }
    cout << count << endl;
    cout << clock() - start_time;
    return 0;
}

The type returned by [`std::string::length`](http://en.cppreference.com/w/cpp/string/basic_string/size) is *unsigned*. Your comparison between signed and unsigned integers likely doesn't work as you expect. — Some programmer dude, Dec 25 '16 at 05:26
And what is the purpose of starting the loop at `-10000`? What is the *actual*, the *original* problem you try to solve? Please [read about the XY problem](http://xyproblem.info/). — Some programmer dude, Dec 25 '16 at 05:29
Another way to fix it would be `for (unsigned int i = 0; i < s.length()+10000; i++)`. In general, I would use `size_t i` here. — barak manos, Dec 25 '16 at 05:35
@barakmanos casting 's.length()' to int solved it. thanks mate — wannabe, Dec 25 '16 at 05:36
No problem, but as @Someprogrammerdude has mentioned, you should think more carefully about what you're trying to achieve. See my comment above for a more vivid way of writing down that `for` loop. — barak manos, Dec 25 '16 at 05:37

score 0 · Answer 1 · answered Dec 25 '16 at 06:45

0

You are running into a typical signed-vs-unsigned comparison failure - one of the frequently asked questions on SO (see Comparison operation on unsigned and signed integers for one example).

You can fix your code by switching to equality comparison instead of relational comparison

for (int i = -10000; i != s.length(); i++)

or by forcing signed integer comparison

for (int i = -10000; i < (int) s.length(); i++)

(although in the latter case you are limiting the range of possible lengths without a good reason).

But in general it is a good idea to avoid mixing signed and unsigned types in comparisons. Redesign your code to avoid it.

answered Dec 25 '16 at 06:45

AnT stands with Russia

312,472
42
525
765

I think that the comment made by @Someprogrammerdude is in fact the best answer in this case, or more accurately, implies what the actual problem "behind the scene" is (i.e., what's wrong with the OP's way of thinking). The `-10000` should be moved from the initial value into the condition itself, i.e., `i – barak manos Dec 25 '16 at 06:55
In addition to all of that - though I'm personally not "harsh" on this kind of issue but many users here are - since the question is tagged C++, you may want to replace that C-style cast with `static_cast`... – barak manos Dec 25 '16 at 06:56
@barak manos: I don't see it as an issue at all. I believe that in C++ code `static_cast` should be reserved to pointer casts: hierarchical up-casts and casts from `void *`. Arithmetic casts should be expressed by C-style casts, exactly as I did above. – AnT stands with Russia Dec 25 '16 at 07:47
I thought the same, which is why I posted [this question](http://stackoverflow.com/q/27525577/1382251). I still generally agree with you, but I got some interesting answers to that question, which you might want to have a look at. – barak manos Dec 25 '16 at 07:50
@barak manos: As for the "best answer"... the code is too abstract to allow any meaningful decision about what is the "best" solution in this case. Until the OP provides a detailed explanation of the context that led them to this strange cycle, there's no way to say what solution is the "best" here. But I don't believe there is any meaningful context here. The code appears to be a contrived experiment. For a code like that trying to prove that one solution is better than the other is rather pointless endeavor. – AnT stands with Russia Dec 25 '16 at 07:51

(c++) for loop is being skipped

1 Answers1