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

Discussion in 'C++' started by Ivar, Nov 17, 2005.

  1. Ivar

    Ivar Guest

    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
     
    Ivar, Nov 17, 2005
    #1
    1. Advertising

  2. Ivar wrote:
    > 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.)
     
    Neelesh Bodas, Nov 17, 2005
    #2
    1. Advertising

  3. Ivar wrote:
    > 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, Nov 17, 2005
    #3
  4. Jonathan Mcdougall wrote:
    > Ivar wrote:
    > > 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
     
    Jonathan Mcdougall, Nov 17, 2005
    #4
  5. Ivar

    shikn Guest

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



    "Ivar" <> wrote in message
    news:...
    > 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
    >
     
    shikn, Nov 17, 2005
    #5
  6. On 2005-11-17 04:43:01 -0500, "Jonathan Mcdougall"
    <> 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);


    --
    Clark S. Cox, III
     
    Clark S. Cox III, Nov 17, 2005
    #6
  7. Clark S. Cox III wrote:
    > On 2005-11-17 04:43:01 -0500, "Jonathan Mcdougall"
    > <> 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."


    Jonathan
     
    Jonathan Mcdougall, Nov 17, 2005
    #7
  8. Ivar

    werasm Guest

    Ivar wrote:
    > 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
     
    werasm, Nov 17, 2005
    #8
  9. Jonathan Mcdougall wrote:
    > Clark S. Cox III wrote:
    >> On 2005-11-17 04:43:01 -0500, "Jonathan Mcdougall"
    >> <> 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."
    >
    >
    > Jonathan
    >


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

    Krishanu
     
    Krishanu Debnath, Nov 17, 2005
    #9
  10. Krishanu Debnath wrote:
    > Jonathan Mcdougall wrote:
    > > Clark S. Cox III wrote:
    > >> On 2005-11-17 04:43:01 -0500, "Jonathan Mcdougall"
    > >> <> 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
     
    Jonathan Mcdougall, Nov 17, 2005
    #10
  11. Jonathan Mcdougall wrote:
    > Krishanu Debnath wrote:
    >> Jonathan Mcdougall wrote:
    >>> Clark S. Cox III wrote:
    >>>> On 2005-11-17 04:43:01 -0500, "Jonathan Mcdougall"
    >>>> <> 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
     
    Krishanu Debnath, Nov 17, 2005
    #11
  12. Ivar

    Ron Natalie Guest

    Ivar wrote:
    > 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;
     
    Ron Natalie, Nov 17, 2005
    #12
  13. Krishanu Debnath wrote:
    > Jonathan Mcdougall wrote:
    > > Krishanu Debnath wrote:
    > >> Jonathan Mcdougall wrote:
    > >>> Clark S. Cox III wrote:
    > >>>> On 2005-11-17 04:43:01 -0500, "Jonathan Mcdougall"
    > >>>> <> 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
     
    Jonathan Mcdougall, Nov 17, 2005
    #13
  14. Ivar

    werasm Guest

    Ron Natalie wrote:
    >
    > 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
     
    werasm, Nov 17, 2005
    #14
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Timothy Madden
    Replies:
    4
    Views:
    384
    Timothy Madden
    Sep 28, 2004
  2. Schnoffos
    Replies:
    2
    Views:
    1,247
    Martien Verbruggen
    Jun 27, 2003
  3. Hal Styli
    Replies:
    14
    Views:
    1,697
    Old Wolf
    Jan 20, 2004
  4. ramadevimandala
    Replies:
    0
    Views:
    530
    ramadevimandala
    Apr 16, 2009
  5. Steve Hicks
    Replies:
    2
    Views:
    1,301
Loading...

Share This Page