Unexpected output in C++

Question

This is not a problem with programming contest but with the language C++.

There is an old programming problem on codeforces. The solution is with C++. I already solved in Python but I don't understand this behavior of C++. In my computer and on onlinegdb's C++ compiler, I get expected output but on codeforces judge, I get a different output.

If interested in the problem : http://codeforces.com/contest/8/problem/A It's very simple and a small read. Though Reading it is not required for the question.

Task in Short:

Print("forward") if string a is found in string s and string b is also found in s

Print("backward") if string a is found in reverse of string s and string b is also found in reverse of s

Print("both") if both of above are true

Print("fantasy") if both of above are false

#include<bits/stdc++.h>
using namespace std;
#define int long long

//initializing all vars because blogs said uninitialized vars sometimes give unexpected result
string s="", a="", b="";
bool fw = false;
bool bw = false;
string now="";
string won="";
int pa=-1, pb=-1, ra=-1, rb=-1;

signed main()
{
    //following 2 lines can be ignored
    ios_base::sync_with_stdio(false);
    cin.tie(NULL);
    //taking main input string s and then two strings we need to find in s are a & b
    cin >> s >> a >> b;
    //need reverse string of s to solve the problem
    string r = s;
    reverse(r.begin(), r.end());
    //pa is index of a if a is found in s else pa = -1 if not found
    pa = s.find(a);

    //if a was a substring of s
    if (pa != -1) {
        //now is substring of s from the next letter where string a was found i.e. we remove the prefix of string till last letter of a
        now = s.substr(pa + a.size(), s.size() - (pa + a.size()));
        //pb stores index of b in remaining part s i.e. now
        pb = now.find(b);

        //if b is also in now then fw is true
        if (pb != -1) {
            fw = true;
        }
    }
    //same thing done for the reverse of string s i.e. finding if a and b exist in reverse of s
    ra = r.find(a);
    if (ra != -1) {
        won = r.substr(ra + a.size(), r.size() - (ra + a.size()));
        rb = won.find(b);
        if (rb != -1) {
            bw = true;
        }
    }
    if (fw && bw) {
        cout << "both" << endl;
    }
    else if (fw && !bw) {
        cout << "forward" << endl;
    }
    else if (!fw && bw) {
        cout << "backward" << endl;
    }
    else {
        cout << "fantasy" << endl;
    }
    return 0;
}

For input

atob
a
b

s="atob", a="a", b="b"

Here reverse of atob is bota.

a is in atob.

So, string now = tob.

b is in tob so fw is true.

Now a is in bota.

So, string won = "" (empty because nothing after a). So, b is not in won.

So, rw is false.

Here answer is to print forward and in C++14 on my PC and onlinegdb, the output is forward but on codeforces judge, it's both.

I did many variations of the code but no result.

Finally I observed that if I run my program on PC and don't give any input and terminate the program in terminal with Ctrl-C, it prints both which is strange as both should only be printed when both fw and rw are true.

What is this behavior of C++?

`pa` etc should be `std::string::size_type` (which is guaranteed to be `size_t`) and `-1` should be `std::string::npos` which definitely isn't guaranteed to be `-1`. — john, May 07 '20 at 13:12
@john the tricky part is that it is defined as an `unsigned` `-1`, so its value isn't `-1` — 463035818_is_not_an_ai, May 07 '20 at 13:14
I expect the actual problem is `#define int long long` which is causes signed/unsigned issues when comparing -1 with the result of `find`. — john, May 07 '20 at 13:17
@idclev463035818 what do you mean by `find ` never returns `-1`. Are you talking about my program or `C++ string`. I checked by printing `pa`. It prints `-1` if string is not found. — Pera, May 07 '20 at 13:18
`find` returns `std::string::npos` when something isnt found see for example [here](https://en.cppreference.com/w/cpp/string/basic_string/npos). That it might print as `-1` is irrelevant. If you want to write portable code use `npos`. — john, May 07 '20 at 13:19
@Pera I really have to ask, what is the thinking behind `#define int long long`. I see this quite often, but it really is a monstrously bad piece of coding. — john, May 07 '20 at 13:21
@john **YES**. You are right. I tried changing `signed main` to `int main` and removing this `#define int long long` and _VOILAAA_, it worked. But I don't understand why? Does signed main restrict returning negative values? Please add this as answer or else I will do it if you want. — Pera, May 07 '20 at 13:22
Never `#include` - Your program is not portable and I can't even compile it. — Ted Lyngmo, May 07 '20 at 13:22
@Pera Great but what you should really do, to make your code work under all circumstances is write `size_t pa;` and `if (pa == std::string::npos)` — john, May 07 '20 at 13:23
BTW: that's 3-4 times the amount of code that's needed, and it's much more complicated than it needs to be. — molbdnilo, May 07 '20 at 13:26
@molbdnilo The problem was that I didn't exactly know, which part of code can create this problem, even if I had the slightest idea, I would have cut it down. — Pera, May 07 '20 at 13:28
That's why the route to a minimal, complete verifiable example involves removing code to _find out_ whether that removal fixes the problem, iteratively narrowing the scope. — Useless, May 07 '20 at 13:32
sorry I was wrong. `find` can return `-1`, however you should not rely on that and use `std::string::npos` instead — 463035818_is_not_an_ai, May 07 '20 at 13:33
@Useless I agree. I made the mistake of not doubting the obvious part of code such as defining and signed main, so I ignored changing them as I thought how can they create a problem. — Pera, May 07 '20 at 13:34
@john Can you please explain `the tricky part is that it is defined as an unsigned -1, so its value isn't -1`. Cause still didn't exactly understand it. — Pera, May 07 '20 at 13:47
@Pera often `size_type` is unsigend, and `npos` is defined as `static const size_type npos = -1;` which looks like it is `-1` but for an `unsigned` `size_type` it is the maximum value of that type. Assigning that to an `int` and compare it to `-1` can fail — 463035818_is_not_an_ai, May 07 '20 at 13:50
@john `I really have to ask, what is the thinki...`. Just because spontaneously I write `int` even when input can be as large as `long long`. So to avoid it I define it as `int` so that range is never an issue an `signed main` because `int` now is `long long`. I checked that the code is accepted with `signed main` and everything else same just by removing `define int long long`. I still don't get why? What's the issue? Please solve it once and for all. — Pera, May 07 '20 at 13:52
_What is this behavior of C++?_ The `#define int long long` makes the rest of the program undefined behavior. Undefined behavior means anything can happen, where anything includes "crashing", or "never terminating", or "producing the wrong output", or "producing the correct output", or "sending your browser history to your grandmother and formatting your hard drive". — Eljay, May 07 '20 at 13:54
@Pera It's immensively confusing to any one reading your code if what looks like an `int` is acutally a `long long`. I didn't spot that originally, but it turned out to be the cause of your error. Sometimes I see `#define ll long long` which is still poor, but at least doesn't redefine a basic type. It shouldn't be your goal to minimise your typing, it should be your goal to write clear code. — john, May 07 '20 at 14:00
@Pera As for the actual problem, C++ has some complicated rules about how integers of different size and signedness interact (when compared or assigned for example). Your code was assigning an unsigned quantity to a signed variable of a different size (long long vs int) and that was enough to mean that the obvious comparison with -1 no longer worked. Basically follow the rules, don't take shortcuts. If the documentation says a variable should be `size_t` then don't use `int` or `long long`. If the documentation says that a return value is `std::string::npos` then use that, not `-1`. — john, May 07 '20 at 14:03

score 4 · Accepted Answer · answered May 07 '20 at 13:50

Let's dissect this code and see what problems we can find. It kind of tips over into a code review, but there are multiple problems in addition to the proximate cause of failure.

#include<bits/stdc++.h>

Never do this. If you see it in an example, you know it's a bad example to follow.

using namespace std;

Fine, we're not in a header and brevity in sample code is a reasonable goal.

#define int long long

Oh no, why would anyone ever do this? The first issue is that preprocessor replacement is anyway prohibited from replacing keywords (like int).

Even without that prohibition, this later line

int pa=-1, pb=-1, ra=-1, rb=-1;

is now a deliberate lie, as if you're obfuscating the code. It would have cost nothing to just write long long pa ... if that's what you meant, and it wouldn't be deceptive.

//initializing all vars because blogs said uninitialized vars sometimes give unexpected result
string s="", a="", b="";

But std::string is a class type with a default constructor, so it can't be uninitialized (it will be default-initialized, which is fine, and writing ="" is just extra noise).

The blogs are warning you about default initialization of non-class types (which leaves them with indeterminate values), so

bool fw = false;

is still sensible.

NB. these are globals, which are anyway zero-initialized (cf).

signed main()

Here are the acceptable faces of main - you should never type anything else, on pain of Undefined Behaviour

int main() { ... }
int main(int argc, char *argv[]) { ... }

Next, these string positions are both (potentially) the wrong type, and compared to the wrong value:

ra = r.find(a);
if (ra != -1) {

could just be

auto ra = r.find(a);
if (ra != std::string::npos) {

(you could write std::string::size_type instead of auto, but I don't see much benefit here - either way, the interface, return type and return values of std::string::find are well-documented).

The only remaining objection is that none of now, won or the trailing substring searches correspond to anything in your problem statement.

`signed main` gets away with it because `int` has implicit `signed int`, and `signed` by itself has an implicit `signed int`. So they're all synonyms. But that was just a workaround for the `#define int` hokum. — Eljay, May 07 '20 at 13:58
Yes. I didn't remove `now` and `won` because `bool` values were changing only in that part so I thought it would look useless to write two nested if statements without something in the middle. Your answer sums it up completely. Kudos. It will be useful for so many who are using some bad code practices for ages without realizing when it can fail. — Pera, May 07 '20 at 14:04

The Long Nguyen · Answer 2 · 2020-05-11T00:58:04.323

The above answer and comments are way more enough information for your question. I cannot comment yet so I would like to add a simplified answer here, as I'm also learning myself.

From the different outputs on different compilers you can trackback the logic and found the flow of code is differ in this line:

if (rb != -1) {

Simply adding a log before that line, or using a debugger:

cout << "rb:" << rb << endl;

You can see that on your PC: rb:-1

But on codeforces: rb:4294967295

won.find(b) return npos, which mean you have an assignment: rb = npos;

This is my speculation, but a possible scenario is:

On your PC, rb is compiled as int (keyword), which cannot hold 4294967295, and assigned to -1.

But on codeforces, rb is compiled as long long, follow the definition, and 4294967295 was assigned instead.

Because you redefine the keyword int, which is advised again by standard of C++ programming language, different compiler will treat this line of code differently.

Unexpected output in C++

2 Answers2