String to Integer

[BoulderMen]

I am trying to write a program that converts a string of numerals to integers. The program I have written gives me an invalid output. Any ideas what I have done wrong ?

//Defining what libraries should be included.
#include <stdio.h>
#include<math.h>

int strIntegerTmp(char *string, int collect);
int strInteger(char *string);

int main (void)
{
    int collect;
    char *string;

    printf("Enter a string of integers: ");
    scanf("%s", &string);
    //length = strlen(string);

    //printf("The integer entered is : %d\n", strInteger(collect));
    printf("The integer entered is : %d\n", strInteger((char *) *string));

    system("PAUSE");
    return 0;
}

int strIntegerTmp(char *string, int collect)
{
    if (!string || !*string || !isdigit(*string))
    {
        return collect;
    }

    else 

         return strIntegerTmp(string + 1, collect * 10 + (*string - '0'));

}

int strInteger(char *string)
{
    return strIntegerTmp(string, 0);
}

[WaltP]

The statement if (!string || !*string || !isdigit(*string)) alone tells me you have no idea how to handle an array of characters. You need to read up on 'strings' and pointers to understand them. Then what you need to do should be clearer.

[deceptikon]

This is wrong:

char *string;
printf("Enter a string of integers: ");
scanf("%s", &string);

There are four bugs here, and one of them is fatal. The fatal bug is that you're assuming an uninitialized pointer somehow points to infinite memory. Pointers must point to memory that your process owns, either by using the address of an existing object, or by allocating memory explicitly. You don't need a pointer here anyway, so just make string an array.

char string[100];

The second bug is a type mismatch. Since string is already evaluated as a pointer in the scanf() call, adding the address-of operator changes the type to a pointer to a pointer to char when scanf() only expects a pointer to char. This invokes undefined behavior because you're lying to scanf().

The third bug is also in the scanf() call and it's a failure to specify a field width for string input. Without a field width, the %s specifier is no better than gets() in terms of buffer overflow safety.

The final bug is a bit subtle, and Windows compilers generally don't exhibit symptoms. But if you want your code ot work everywhere, always flush stdout when displaying prompts that don't end in a newline. There are three times when an output stream is flushed (and for stdout that typically means the string shows up on your monitor):

1. When the buffer is full.
2. When a newline is printed.
3. When fflush() is called.

You don't control the buffer being full, and it's not always desirable to print a newline, so the only remaining option is fflush().

You could also call not checking for failure a bug, but that's probably nitpicking on my part. ;) Here's how the fixed main() looks (with error checking added too):

int main(void)
{
    char string[100];

    printf("Enter a string of integers: ");
    fflush(stdout);

    if (scanf("%99s", string) == 1)
        printf("The integer entered is: %d\n", strInteger(string));

    return 0;
}

There's technically nothing wrong with your string to int conversion, aside from being totally unsafe in terms of range checking, complete failure to negative values, and my own personal belief that recursion is ill suited to linear problems.

By the way, you need to include <ctype.h> for isdigit().

The statement if (!string || !string || !isdigit(string)) alone tells me you have no idea how to handle an array of characters.

Could you elaborate? It looks fine to me. If the pointer is null, or the first character in the string is a null character, or the first character in the string is not a digit then don't try to convert the character. It's not perfect, but for a naive conversion I don't see any problems.

[VatooVatoo]

This is working code on Watcom C/C++

#include <stdio.h>
#include <stdlib.h> 
#include <ctype.h> 
#include <math.h>
#include <string.h> 

int iConvertString2Integer(char *string, int iIndex, int iIntegerValue);
int iString2Integer(char *string);

int main (void)
{
    char string[512];
    printf("Enter a string of integers: ");
    scanf("%s\0", string);

    printf("The integer entered is : %d\n", iString2Integer((char *)string));
    system("PAUSE");
    return 0;
}

int iString2Integer(char *string)
{
    return iConvertString2Integer(string, strlen(string)-1, 0);
}

int iConvertString2Integer(char *string, int iIndex, int iIntegerValue)
{
    if (*string=='\0' || !isdigit(*string))
    {
        return iIntegerValue;
    }
    else
    {
        iIntegerValue += (*string-'0') * ((int)pow( 10, iIndex ));
        return iConvertString2Integer(string + 1, --iIndex, iIntegerValue);
    }
}

[VatooVatoo]

I did not do any HW for others. He tried and need help and I get him a way to know what is his wrong. There were several mistakes in his code and I do it for him.

[WaltP]

I did not do any HW for others. He tried and need help and I get him a way to know what is his wrong. There were several mistakes in his code and I do it for him.

You said:

This is working code on Watcom C/C++

so yes, you did.
Correcting all his mistakes without pointing out what they are is doing his work.

Simply pointing out his mistakes and suggesting corrections -- no code -- is helping.

Remember, "I do it for him." is doing it for him, not letting him do it.