HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Output int from string input

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
outputinputfromintstring

Problem

This takes a string input like 1,2,3,4,5, splits it using , delimiter, converts each char to int and output it in a single line like this : 12345

#include 
#include 
#include 

int* createIntArray(char* numbersInString, int* array_size);

int* createIntArray(char* numbersInString, int* array_size)
{
    char* token = strtok(numbersInString,",");
    int* numbersArray = malloc(sizeof(int)*100);
    int i = 0;

    while(token != NULL)
    {
        numbersArray[i] = (int)*token -'0';
        token = strtok(NULL,",");
        i++;
    }

    *array_size = i;
    return numbersArray;
}

int main(){
    char* stringNumbers = malloc(sizeof(char)*100);
    int* intArray;
    int arrayLength = 0;
    printf("Enter numbers\n");
    scanf("%s",stringNumbers);
    intArray = createIntArray(stringNumbers,&arrayLength);

    int i;
    for(i = 0; i < arrayLength;i++)
    {
        printf("%d",intArray[i]);
    }

    free(stringNumbers);
    free(intArray);
    return 0;
}


I know the code is super useless, but I'm starting to learn C and I wanted to see if my code was alright and if it complies to C coding standards.

Solution

-
Since you have main() below the other function, you don't need the prototype for that function. It's only needed if main() is defined before a function that it calls.

-
There's no need to declare a variable and then assign to it:

int* intArray;
// ...
intArray = createIntArray(stringNumbers,&arrayLength);


Variables should just be initialized in the closest scope possible. This will ease maintenance as you won't have to search for the declared variable elsewhere, such as if it's no longer in use.

int* intArray = createIntArray(stringNumbers,&arrayLength);


-
Instead of incrementing i in createIntArray(), why not just increment array_size?

Plus, i is more commonly used as a for loop counter, which isn't the case here. If someone were to take a glance at that while loop without further context, they may think that it should be a for loop instead. By making this change, you'll remove that possible misconception and better communicate your exact intentions.

Code Snippets

int* intArray;
// ...
intArray = createIntArray(stringNumbers,&arrayLength);
int* intArray = createIntArray(stringNumbers,&arrayLength);

Context

StackExchange Code Review Q#57257, answer score: 6

Revisions (0)

No revisions yet.