Revers went wrong and returns the same number

Question

I think i cant understand something in the logic of my cycles, cant u match my mistake? We need to reverse the number. My code:

#include <iostream>
#include <string>

using namespace std;

int main(){
  string a;
  char b;
  cin >> a;
  for (int i = 0; i < a.size(); i++){
    for (int j = a.size(); j > 0; j--){
      b = a[i];
      a[i] = a[j];
      a[j] = b;
      break;
    }
  }
  cout << a;
}

why are there two loops? Why do you think the code should be correct? I do not see that. Try to explain the logic to your [rubber duck](https://en.wikipedia.org/wiki/Rubber_duck_debugging) or use a debugger. — 463035818_is_not_an_ai, Sep 26 '22 at 17:56
There are lots of errors. You only need a single loop. Just print your swap-routine and observe what happens. Your addressing is also wrong: 'j' shouldn't be a.size() but smaller. — m2j, Sep 26 '22 at 17:57
to reverse in place you do not need a loop from first till last character. If you swap each character twice you end up with the original string — 463035818_is_not_an_ai, Sep 26 '22 at 17:58
[`std::reverse`](https://en.cppreference.com/w/cpp/algorithm/reverse) — 463035818_is_not_an_ai, Sep 26 '22 at 17:58
Welcome to Stack Overflow! It is never too early to [learn how to run your code in a debugger](https://stackoverflow.com/questions/25385173). Nobody writes perfect code. A debugger will show you what is wrong. — Drew Dormann, Sep 26 '22 at 18:06

score 1 · Answer 1 · answered Sep 26 '22 at 18:08

The problem is your nested loop do the swap operation too many times(n*n times).

You just need a single loop to achieve that.

And also remind you the j should be a.size() - 1 and j >= 0, because the array index is start with 0.

#include <iostream>

int main()
{
    std::string a;
    char b;
    std::cin >> a;
    for(std::string::size_type i = 0; i < a.size() / 2; ++i)
    {
        b = a[i];
        a[i] = a[a.size() - 1 - i];
        a[a.size() - 1 - i] = b;

    }
    std::cout << a << "\n";
}

You can consider a even more readable one.

#include <iostream>
#include <algorithm>

int main()
{
    std::string a;
    std::cin >> a;
    std::reverse(std::begin(a), std::end(a));
    std::cout << a << "\n";
}

Vlad from Moscow · Answer 2 · 2022-09-26T18:11:49.837

0

These nested for loops

  for (int i = 0; i < a.size(); i++){
    for (int j = a.size(); j > 0; j--){
      b = a[i];
      a[i] = a[j];
      a[j] = b;
      break;
    }
  }

do not make sense.

For example the inner for loop has only one iteration due to the break statement within it. And this assignment

a[j] = b;

invokes undefined behavior because it is equivalent to

a[a.size()] = b;

It is enough to use only one for loop as for example

for ( std::string::size_type i = 0, n = a.size(); i < n / 2; i++ )
{
    std::swap( a[i], a[n-i-1] );
}

If you do not know yet the standard function std::swap then you can rewrite the body of the for loop the following way

for ( std::string::size_type i = 0, n = a.size(); i < n / 2; i++ )
{
    auto c = a[i];
    a[i] = a[n-i-1];
    a[n-i-1] = c; 
}

edited Sep 26 '22 at 18:11

answered Sep 26 '22 at 18:02

Vlad from Moscow

301,070
26
186
335

Even this might be overkill. If they just need to print it reversed, you don't need to bother reversing it in storage first. – sweenish Sep 26 '22 at 18:03
@sweenish To reverse a string and to output a string in the reverse order are two different tasks. He wants to reverse a string. – Vlad from Moscow Sep 26 '22 at 18:05
Yes, they are, and I doubt that they need to actually reverse it. Looking at the presented code, I think if it just prints it in reverse order, it'd be fine. I also wonder if this wasn't supposed to be an exercise in recursion. – sweenish Sep 26 '22 at 18:11

Revers went wrong and returns the same number

2 Answers2