Member Avatar for NeoFryBoy

Quick background: I'm working on Project Euler and made a palindrome checker. I needed to convert integers to strings. itoa() kept giving random values (9 gave 9 or 90, etc...), so I decided to make my own, and ran into a problem. I have fixed it, but could somebody explain what caused the problem and why the fix works?

Broken Code: The below code creates a stringstream inputs the number then puts that into a string and then into a char. The problem is it would chop the last number off when returning the char. (The buffer holds the 99800 value but the receiving variable numaschar = strItoA(num); becomes 9980.)

char* strItoA(int number)
{
   std::ostringstream sin;
   sin << number;
   std::string val = sin.str();

  size_t length;
  char buffer[val.size()+1]; //add one for the zero terminator
  length=val.copy(buffer,val.length(),0); //do the transfer and grab the last position
  buffer[length]='\0'; //zero terminate!!
  return buffer;
}

The Fix:

char* buffer; //moved outside of function and given reference

char* strItoA(int number)
{
//Same...

  buffer = new char [val.size()+1]; //added to keep buffer dynamic
//char buffer[val.size()+1]; //removed
//Same...
}

Thanks. :)

William Hemsworth commented: Perfect first post. +13

Recommended Answers

All 6 Replies

Curious. You should use std::string s really...

std::string strItoA(int number) {
   std::ostringstream sin;
   sin << number;
   return sin.str();
}

Though Narue may tell you to generalise (which is a good idea, even though she spells it wrong ;)) which is where templates become your friend.

.

Member Avatar for NeoFryBoy

Curious. You should use std::string s really...

std::string strItoA(int number) {
   std::ostringstream sin;
   sin << number;
   return sin.str();
}

Though Narue may tell you to generalise (which is a good idea, even though she spells it wrong ;)) which is where templates become your friend.

That doesn't help at all. But thanks for the attempt.

Quick background: I'm working on Project Euler and made a palindrome checker. I needed to convert integers to strings. itoa() kept giving random values (9 gave 9 or 90, etc...), so I decided to make my own, and ran into a problem. I have fixed it, but could somebody explain what caused the problem and why the fix works?

Broken Code: The below code creates a stringstream inputs the number then puts that into a string and then into a char. The problem is it would chop the last number off when returning the char. (The buffer holds the 99800 value but the receiving variable numaschar = strItoA(num); becomes 9980.)

char* strItoA(int number)
{
   std::ostringstream sin;
   sin << number;
   std::string val = sin.str();

  size_t length;
  char buffer[val.size()+1]; //add one for the zero terminator
  length=val.copy(buffer,val.length(),0); //do the transfer and grab the last position
  buffer[length]='\0'; //zero terminate!!
  return buffer;
}

The Fix:

char* buffer; //moved outside of function and given reference

char* strItoA(int number)
{
//Same...

  buffer = new char [val.size()+1]; //added to keep buffer dynamic
//char buffer[val.size()+1]; //removed
//Same...
}

Thanks. :)

Looking briefly at the first block of code, I think it's because the char buffer is declared inside your function and only has scope inside your function. So as soon as your function returns, the object referenced by the returned value has been invalidated as it is out of scope. In which case you're lucky to have anything sensible at all in the return value. That or perhaps you were overwriting the last character in the char array at line 10, where you're adding the null terminator.
I think the calculation of length in line 9 is incorrect as it holds the number of characters in the array (excluding the null terminator) so changing line 10 to:

buffer[val.size()]='\0';

May fix the problem...
You've already stated in your code that the buffer should be the size of val.size()+1. after copying the contents of val into the buffer, the string in the buffer will be the same size as the string in val. So the item in the buffer at the array position buffer[val.size()] must therefore be the null terminator.

Remember: You've reserved val.size()+1 spaces for buffer and in c++, arrays start at position 0. Therefore the array position at the index val.size() must be the position reserved for the null terminator!

However in your first block of code, all of that stuff copying into the char buffer strikes me as unnecessary, plus there is the scope problem. I think all you needed to do was this:

char* strItoA(int number)
{
   std::ostringstream sin;
   sin << number;

   return new std::string(sin.str()).c_str();
   // The above line is equivalent to:
   // std::string val = new std::string(sin.str());
   // return val.c_str();
   // c_str() returns a c-style string (char*) from a std::string
   // you can also get a char* from an ostringstream
   // e.g.    sin.str().c_str()
}

Hope this is of some help.
Cheers for now,
Jas.

Member Avatar for NeoFryBoy

Hmmm I had thought return would dump the value into a new variable before deleting the old one.

I had extended the buffer size at one point so I'm sure I wasn't overwriting.

Well, thanks for suggesting the unique usage of c_str(). I had seen it used before, but they used strcpy() along with it and I was told not to use that function.

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.