4

I'm writing a function to return the reverse of a number i.e it converts int(1234) to int(4321). This is what I have currently:

#include <iostream>
#include <cstdlib>
#include <string>
#include <sstream>

using namespace std;


int reverse(int num) {
  stringstream ss (stringstream::in | stringstream::out);
  string initial; 
  int reversed;

  // read the number in to a string stream
  ss << num;
  initial = ss.str();

  // flush the stringstream
  ss.str("");
  for(unsigned int i(0); i <= initial.size(); i++) {
    ss << initial[initial.size() - i];
  }
  ss >> reversed;

  return reversed;
}


int main(int argc, const char *argv[])
{
  int test = 9871;
  cout << "test = " << test << endl;
  cout << "reverse = " << reverse(test) << endl;

  return 0;
}

However this just outputs:

test = 9871
reverse = 0

And I'm pretty certain the problem is in the line ss >> reversed is the problem in that reversed is being set to 0 instead of the value of ss, but I can't figure out what's wrong with this code, and it's infuriating as it seems like it should be simple. Can anyone help?

Thanks

Improve this question
1
  • 3
    Not an answer to your problem, but as a general rule, it's best to avoid reusing a stringstream. In your case, I'd use an std::ostringstream to get the string, std::reverse on it, and an std::istringstream to get the reversed value. Commented May 30, 2012 at 15:43

5 Answers 5

Reset to default
5

i starts from 0, then initial.size() - i is out of string bounds.

Change to ss << initial[initial.size() - i - 1]; and iterate i from 1 to initiali.size() - 1

for(unsigned i = 0; i != initial.size(); ++i) {
  ss << initial[initial.size() - i -1];
}

Or iterate i from 1 to initial.size()

for(unsigned i = 1; i <= initial.size(); ++i) {
  ss << initial[initial.size() - i];
}
Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

Without changing the terminating condition in the for, this change alone still results in out of bounds.
I guess this is homework... I suggest to improve your C++ style (declare variables just before you use them)
this works: for(unsigned int i = 0; i < initial.size(); i++) { ss << initial[initial.size() - i - 1]; }
Doing for(unsigned int i = 1; i <= initial.size(); i++) { ss << initial[initial.size() - i]; } also works.
2

The for loop results in out of bounds access on initial. In addition to problem pointed out by Alessandro Pezzato the terminating condition in the for loop needs changed to i < initial.size() otherwise an out of bounds will still occur:

for(unsigned int i(0); i < initial.size(); i++) {
    ss << initial[initial.size() - i - 1];
}
Improve this answer

2 Comments

Yea, I didn't notice the <=. And this insidious because it seems to work even with this error. +1
@AlessandroPezzato, the arena of undefined behaviour.
2

The quickest C++11 way to perform what you want is:

string s = std::to_string(my_int);
std::reverse(begin(s), end(s));
return std::stoi(s);
Improve this answer

3 Comments

I suspect std::string s=std::to_string(my_int); return std::stoi(std::string(s.rbegin(), s.rend())); would be at least as quick.
@JerryCoffin I doubt that it would make a significant difference, but using reverse iterators and the two iterator constructor is very idiomatic C++. (On the other hand, one might argue that using std::reverse when you want to reverse something might be clearer. YMMV, as they say.)
And in pre C++11, you can still use either std::reverse or Jerry's solution with std::[io]stringstream.
2

Your problem stems from the way you index into your array, you always need to know in the back of your head that indexing arrays in C/C++ and many similar languages is zero-based because it allows for some counting issues to go away common when starting at one.

If the size of your string is 16 characters, that means that indices of that particular string max out at 16-1=15, giving the range [0,15]. In general, it's size() - 1. If initial[initial.size() - 1 - i] looks dirty to you, you can always set it into a temporary variable like maxIndex. 

int maxIndex = initial.size() - 1;
for(unsigned int i = 0; i <= maxIndex; i++) 
{
    ss << initial[maxIndex - i]; 
}
Improve this answer

Comments

0

It seems to me that a string conversion makes this more complex. I think I'd do a direct conversion from int to int:

int reverse(int input) {
    static const int base = 10;
    int ret = 0;

    while (input) { 
        ret = ret * base + input % base;
        input /= base;
    }
    return ret;
}

Note that you'd have to get (a tiny bit) more elaborate to handle negative numbers correctly. I used int as the input and output type because the original did -- but like the original this only really produces sensible results for non-negative inputs.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.