Argthtjtr

So I've been working on a program that invert it's characters. And I've doing it by string.length() and string.at(). But, it occur error "out of range" even though it's not out of range (I've checked it by print the pos of at variabele) I suspected it's caused of data types mismatch. Correct me if I'm wrong. This is my code.

 #include "pch.h"

#include <iostream>

#include <conio.h>

#include <string>



using namespace std;



string compose(string temp) {

    size_t temp1 = 0, temp2 = temp.length() - 1;

    string temporary, temporary1;

    cout << temp2;

    while (temp2 < temp.length() / 2 + 1 || temp2 > temp.length() / 2 - 1) {

        temporary1 = temp.at(temp1);

        temporary = temp.at(temp2);



        temp.replace(temp1, 1, temporary);

        temp.replace(temp2, 1, temporary1);

        temp1++;

        temp2--;

    }

    return temp;

}



int main()

{



    cout << compose("RPL X-1okewphevoiwrwrejfnjjjjjjjjjjjjjjjjjnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn");

    _getch();

    return 0;

}

Change this:

while (temp2 < temp.length() / 2 + 1 || temp2 > temp.length() / 2 - 1)

to:

while (temp2 < temp.length() / 2 - 1 || temp2 > temp.length() / 2 - 1)

Explanation:

I've checked it by print the pos of at variabele

Your check was insufficient. I did a:

cout << temp1 << " " << temp2 << endl;

at the first line of the body of the while loop, and it was obvious that temp2 was underflowing, which explains the out of range error.

What you did was to force a size_t (imagine it as an unsigned integer) variable go below 0. Read more in Question about C behaviour for unsigned integer underflow.

In order to fix this, you need to understand what allows your temp2 to underflow. At the end of your loop, you do temp2--;,, which decrements temp2 by 1.

You should control how many times this gets executed, by having a stop condition in your while loop. Thats approach allows us to focus on the stop condition in the while loop.

With an input like "bar", and not that big string you use there, you can easily see, that by the time it gets inverted into "rab", temp1 is equal to 2 and temp2 is equal to 0.

If you allow the body of the loop of to get executed once more, you will then allow temp2 to underflow, invoking Undefined Behavior (UB), when you try to use temp2.

Checking the stop condition and the values of your counters, you can see that you need to change the first operand of the OR condition, as explained above.

Homework: Think how you can simplify your stop condition. Take a small string as an example input and see how counters temp1 and temp2 update themselves.

Hint: One condition will suffice (you do not need two conditions paired with a logical OR).

PS: Since we discussed about underflow, it should be natural now for someone to think that temp must be ensured that is not empty. If it is, then this temp2 = temp.length() - 1; would result in temp2 to underflow!

So, modify your function to do in its first line if(temp.length() == 0) return "";.

The error is here:

while (temp2 < temp.length() / 2 + 1 || temp2 > temp.length() / 2 - 1) {

                                 ^-- should be -1

But actually the check itself is wrong. In theory correct would be:

while (temp1 < temp.length() / 2 - 1 || temp2 > temp.length() / 2 - 1) {

           ^-- change here

But even this is too much. Just checking

while (temp1 < temp.length() / 2 - 1)

Should be enough. The reason is you have temp1 and temp2 where one starts from 0 and the other one from the end of the string. Each step you increase temp1 and decrease temp2. You have to distinguish 2 cases:

• string length is even


• string length is odd

But for both cases it's enough to just check one of the variables if they are about to pass halfway through the string (even length string) or about to hit the middle character (odd length string) where a switch is pointless.

And as Lightness Races in Orbit mentions in the comments you have to make sure the passed string is not empty. But in that case you can just return.

This one should be enough (little bit KISS):

#include <iostream>

#include <string>



using namespace std;



string compose(string temp) {

    size_t temp1 = 0, temp2 = temp.length() - 1;

    string temporary, temporary1;



    size_t size = temp.length() /2;

    size = size %2 == 0? size: size -1;

    while (temp1 <= size) {

        temporary1 = temp.at(temp1);

        temporary = temp.at(temp2);



        temp.replace(temp1, 1, temporary);

        temp.replace(temp2, 1, temporary1);

        temp1++;

        temp2--;

    }

    return temp;

}



int main()

{



    cout << compose("RPL X-1okewphevoiwrwrejfnjjjjjjjjjjjjjjjjj123456789");

    cout << compose("1234");    

    cout << compose("12345");    

    return 0;

}