Please help with bug in my function - converts int to string

[Ivar]

Hi guys - So basically I am trying to implement a function that
converts an int to a string, but it is not working for some reason -
any thoughts? My function, intToStr, is shown below. I'm just trying to
implement this to gain practice with c-style strings.

#include<iostream>
#include"testString.h"

int main(int argc, char* argv[]) {
char* c2 = new char[];
testString::intToStr(c2, -254);
cout << c2 << endl;
delete c2;
c2 = NULL;
return 0;*/
}


void testString::intToStr(char str[], int number) {
int x = number;
if(x < 0)
x = -x;
int order = 0;
while(x > 0) {
x = x/10;
order++;
}
char* tmp = new char[order+2];
tmp[0] = '\0';
int y = number;
for(int i=1; i <= order; i++) {
tmp = (char)(y%10);
y = y/10;
}
if(number < 0)
tmp[order+1] = '-';
else
tmp[order+1] = '+';
testString::reverseString(tmp); /*reverseString works - there is no
bug in that code*/
while(*str++ = *tmp++);
delete tmp;
tmp = NULL;
}

Thanks

 

[Neelesh Bodas]

testString::reverseString(tmp); /*reverseString works - there is no
bug in that code*/

but there is a bug in your usage. since the first character of tmp is
null character, I suspect whether the string will be reversed (provided
reverseString doesnot do anything unusual.)

 

[Jonathan Mcdougall]

Hi guys - So basically I am trying to implement a function that
converts an int to a string, but it is not working for some reason -
any thoughts? My function, intToStr, is shown below. I'm just trying to
implement this to gain practice with c-style strings.

Many things don't work here.

#include<iostream>
#include"testString.h"

int main(int argc, char* argv[]) {
char* c2 = new char[];

Illegal, you must specify an array size here.

testString::intToStr(c2, -254);
cout << c2 << endl;
delete c2;

Illegal. This should be

delete[] c2;

c2 = NULL;
return 0;*/

Remove that */

}


void testString::intToStr(char str[], int number) {
int x = number;
if(x < 0)
x = -x;

Use std::abs().

int order = 0;
while(x > 0) {
x = x/10;
order++;
}
char* tmp = new char[order+2];
tmp[0] = '\0';
int y = number;

Watch out! y should be absolute here or you'll get negative values!

for(int i=1; i <= order; i++) {
tmp = (char)(y%10);

This should be

tmp = '0' + (y%10);

if you want characters. This will only work on ASCII machines.

y = y/10;
}
if(number < 0)
tmp[order+1] = '-';
else
tmp[order+1] = '+';
testString::reverseString(tmp); /*reverseString works - there is no
bug in that code*/
while(*str++ = *tmp++);

Nooo! You just lost the pointer to the memory you allocated. Save it
*before*

char *to_delete = tmp;

delete tmp;

This should crash the application because you are not deleting from the
correct address (tmp has moved in your loop).

tmp = NULL;
}

I think there was some other errors as well, but start by fixing these.
By the way, I understand you are doing that for fun, but you should use
std::istringstream instead:

# include <sstream>

int main()
{
int i = 0;
std::istringstream iss("-254");
iss >> i;
}

Thanks

Jonathan

 

[Jonathan Mcdougall]

char* tmp = new char[order+2];

delete tmp;

This should crash the application because you are not deleting from the
correct address (tmp has moved in your loop).

What's more, you should do

delete[] tmp;

as in main().

int *i = new int;
delete i;

int *i = new int[10];
delete[] i;


Jonathan

 

[shikn]

tmp = (char)(y%10)+ '0'; ?

Hi guys - So basically I am trying to implement a function that
converts an int to a string, but it is not working for some reason -
any thoughts? My function, intToStr, is shown below. I'm just trying to
implement this to gain practice with c-style strings.

#include<iostream>
#include"testString.h"

int main(int argc, char* argv[]) {
char* c2 = new char[];
testString::intToStr(c2, -254);
cout << c2 << endl;
delete c2;
c2 = NULL;
return 0;*/
}


void testString::intToStr(char str[], int number) {
int x = number;
if(x < 0)
x = -x;
int order = 0;
while(x > 0) {
x = x/10;
order++;
}
char* tmp = new char[order+2];
tmp[0] = '\0';
int y = number;
for(int i=1; i <= order; i++) {
tmp = (char)(y%10);

// the above statement maybe need some change? tmp = (char)(y%10)+ '0';

y = y/10;
}
if(number < 0)
tmp[order+1] = '-';
else
tmp[order+1] = '+';
testString::reverseString(tmp); /*reverseString works - there is no
bug in that code*/
while(*str++ = *tmp++);
delete tmp;
tmp = NULL;
}

Thanks

 

[Clark S. Cox III]

for(int i=1; i <= order; i++) {
tmp = (char)(y%10);

This should be

tmp = '0' + (y%10);

if you want characters. This will only work on ASCII machines.

No, it will work on all machines. The characters for the digits '0'
through '9' are guaranteed to be sequential. That is, the following
will *always* produce the character '5':

putc('0' + 5, stdout);

 

[Jonathan Mcdougall]

for(int i=1; i <= order; i++) {
tmp = (char)(y%10);

This should be

tmp = '0' + (y%10);

if you want characters. This will only work on ASCII machines.

No, it will work on all machines. The characters for the digits '0'
through '9' are guaranteed to be sequential. That is, the following
will *always* produce the character '5':

putc('0' + 5, stdout);

Any reference? All I could find is

2.13.2.1 "[...] An ordinary character literal that contains a
single c-char has type char, with value equal to the numerical value of
the encoding of the c-char in the execution character set."


Jonathan

 

[werasm]

Hi guys - So basically I am trying to implement a function that
converts an int to a string, but it is not working for some reason -
any thoughts? My function, intToStr, is shown below. I'm just trying to
implement this to gain practice with c-style strings.

#include<iostream>
#include"testString.h"

int main(int argc, char* argv[]) {
char* c2 = new char[];

How large an array should be allocated? Is it really necessary to make
use of the heap in this case. Why not just use:

char resultStr[100];

testString::intToStr(c2, -254);
cout << c2 << endl;
delete c2;

Bug here. Calling new[] requires you to call delete [], for example:

char* result = new char[x];
....requires...
delete []result;

c2 = NULL;

In these circumstances, this is really not necessary, as c2 is never
accessed again.

return 0;*/
}

For a function like this, I would consider providing a std::string as
argument. If not, I would explicitly indicate the result buffers size
to prevent overflow.

void testString::intToStr(char str[], int number) {
int x = number;
if(x < 0)
x = -x;

IMO the line here above will not give you the desired effect. Maybe
abs( x ) where abs gives you the absolute value of x, else try x *= -1;

int order = 0;
while(x > 0) {
x = x/10;
order++;
}
char* tmp = new char[order+2];

Why always using memory allocated on the heap?- expensive, you know...
Also, is it at all necessary to create a temporary here?

tmp[0] = '\0';

I'll assume reverseString ignores the fact that the string is
null-terminated. If this is the case, it is not string flavoured
(traditionally) at all. Null terminating your first character doesn't
make sense at all.

int y = number;
for(int i=1; i <= order; i++) {
tmp = (char)(y%10);
y = y/10;
}
if(number < 0)
tmp[order+1] = '-';
else
tmp[order+1] = '+';
testString::reverseString(tmp); /*reverseString works - there is no
bug in that code*/

Yes, terminating your strings first character effectively makes it
empty . Reversing an empty strings gives you, well, an empty string
.

while(*str++ = *tmp++);

Hmmm, how about std::copy or memcpy here. Functions are there to be
used. Complex functions are made of less complex ones. Why did you not
write or own memcpy then, and use that if you want to have some
practice at doing it right;

delete tmp;

Ooops, delete []tmp; This may crash...

tmp = NULL;

Hmmm, not necessary.

}

Thanks

Pleasure,

W

 

[Krishanu Debnath]

Ivar wrote:

for(int i=1; i <= order; i++) {
tmp = (char)(y%10);
This should be

tmp = '0' + (y%10);

if you want characters. This will only work on ASCII machines.

No, it will work on all machines. The characters for the digits '0'
through '9' are guaranteed to be sequential. That is, the following
will *always* produce the character '5':

putc('0' + 5, stdout);

Any reference? All I could find is

2.13.2.1 "[...] An ordinary character literal that contains a
single c-char has type char, with value equal to the numerical value of
the encoding of the c-char in the execution character set."


Jonathan

You are looking in the wrong place, try 2.2 character sets.

Krishanu

 

[Jonathan Mcdougall]

On 2005-11-17 04:43:01 -0500, "Jonathan Mcdougall"
<[email protected]> said:

Ivar wrote:

for(int i=1; i <= order; i++) {
tmp = (char)(y%10);
This should be

tmp = '0' + (y%10);

if you want characters. This will only work on ASCII machines.
No, it will work on all machines. The characters for the digits '0'
through '9' are guaranteed to be sequential. That is, the following
will *always* produce the character '5':

putc('0' + 5, stdout);

Any reference? All I could find is

2.13.2.1 "[...] An ordinary character literal that contains a
single c-char has type char, with value equal to the numerical value of
the encoding of the c-char in the execution character set."

You are looking in the wrong place, try 2.2 character sets.

Well I don't have the standard (I will, someday), but there's nothing
in the draft at 2.2, except the characters accepted in a source file.
If there are more explanations in the standard, would it be possible
for someone to quote it here?

Thank you,


Jonathan

 

[Krishanu Debnath]

Clark S. Cox III wrote:
On 2005-11-17 04:43:01 -0500, "Jonathan Mcdougall"
<[email protected]> said:

Ivar wrote:

for(int i=1; i <= order; i++) {
tmp = (char)(y%10);
This should be

tmp = '0' + (y%10);

if you want characters. This will only work on ASCII machines.
No, it will work on all machines. The characters for the digits '0'
through '9' are guaranteed to be sequential. That is, the following
will *always* produce the character '5':

putc('0' + 5, stdout);
Any reference? All I could find is

2.13.2.1 "[...] An ordinary character literal that contains a
single c-char has type char, with value equal to the numerical value of
the encoding of the c-char in the execution character set."

You are looking in the wrong place, try 2.2 character sets.

Well I don't have the standard (I will, someday), but there's nothing
in the draft at 2.2, except the characters accepted in a source file.
If there are more explanations in the standard, would it be possible
for someone to quote it here?

Thank you,


Jonathan

2.2 Character sets

para 3

"For each basic execution character set, the values of the members shall
be non-negative and distinct from one another. In both the source and
execution basic character sets, the value of each character after 0 in
the above list of decimal digits shall be one greater than the value
of the previous."

Krishanu

 

[Ron Natalie]

Hi guys - So basically I am trying to implement a function that
converts an int to a string, but it is not working for some reason -
any thoughts? My function, intToStr, is shown below. I'm just trying to
implement this to gain practice with c-style strings.

Why? You need more practice. Char* is NOT A STRING TYPE.

Try writing this whole thing with std::string first and then once
you get your character handling debugged, you can work at converting
it to dynamically allocated chars if you must.

#include<iostream>
#include"testString.h"

int main(int argc, char* argv[]) {
char* c2 = new char[];

Not legal. Persumably you wanted new char[0], although why I don't know.

testString::intToStr(c2, -254);
cout << c2 << endl;

This is always going to print nothing (but the newline). There
is nothing testString can do to change c2 (and *c2 isn't valid).

delete c2;

Also bogus. If you allocate with new [], you must delete with delete[]:
delete [] c2;

c2 = NULL;
Unnecessary.

void testString::intToStr(char str[], int number) {

int order = 0;
while(x > 0) {
x = x/10;
order++;
}

x != 0 would be clearer, x is never going to be less than zero.

tmp = (char)(y%10);

This almost certainly doesn't do what you want. char is just
an integral type. The unnecessary cast here does NOT convert
the value of y into a character representing that number (it
in fact does NOTHING, there is already an implicit conversion
from int to char).

The following is slighly funky buy you want:

tmp = "0123456789"[y%10];
or (the following is strictly legal, but a little hokie in my
opinion):
tmp = '0' + (y%10);

while(*str++ = *tmp++);

Here is where you are breaking the law. str is not big enough to
hold tmp. You are writing off the end of your allocation.

delete tmp;
tmp = NULL;

Same problem as before, use delete[]tmp and skip the unnecessary assignment.

What you probably want to do is just return tmp (without deleting it)
and assign that to a char* in your main program. Of course your main
program will now be resposible for calling delete[] on that value.
That is it should look like:

char* intToString(int);

char* rv = intToString(-254);
cout << rv << "\n";
delete [] rv;

 

[Jonathan Mcdougall]

Jonathan Mcdougall wrote:
Clark S. Cox III wrote:
On 2005-11-17 04:43:01 -0500, "Jonathan Mcdougall"
<[email protected]> said:

Ivar wrote:

for(int i=1; i <= order; i++) {
tmp = (char)(y%10);
This should be

tmp = '0' + (y%10);

if you want characters. This will only work on ASCII machines.
No, it will work on all machines. The characters for the digits '0'
through '9' are guaranteed to be sequential. That is, the following
will *always* produce the character '5':

putc('0' + 5, stdout);
Any reference? All I could find is

2.13.2.1 "[...] An ordinary character literal that contains a
single c-char has type char, with value equal to the numerical value of
the encoding of the c-char in the execution character set."

You are looking in the wrong place, try 2.2 character sets.

Well I don't have the standard (I will, someday), but there's nothing
in the draft at 2.2, except the characters accepted in a source file.
If there are more explanations in the standard, would it be possible
for someone to quote it here?

Thank you,


Jonathan

2.2 Character sets

para 3

"For each basic execution character set, the values of the members shall
be non-negative and distinct from one another. In both the source and
execution basic character sets, the value of each character after 0 in
the above list of decimal digits shall be one greater than the value
of the previous."

Ah, well, good to know. Thanks,


Jonathan

 

[werasm]

tmp = "0123456789"[y%10];

Hmmm, I like...

What you probably want to do is just return tmp (without deleting it)
and assign that to a char* in your main program. Of course your main
program will now be resposible for calling delete[] on that value.

I know you probably know, but...

One would want to make it more explicit to the caller that he is
responsible for deletion of the return value. For this reason I would
not return char* in this case.

The easiest thing would be to return std::string.

Another option would be to return std::auto_ptr<std::vector<char> >. I
suppose you would know why . The concept of auto_array<char> also
springs to mind. The string, especially considering COW semantics are
probably the best, though.

The OP would still get the practice required (even more so).

That is it should look like:

char* intToString(int);

char* rv = intToString(-254);
cout << rv << "\n";
delete [] rv;

Now becomes:

std::string s( intToStr(24) );
std::cout << s << std::endl;

Kind regards,

Werner