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

Splitting string by words to new strings

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

Problem

Function:

char **strsplit (char source[])
{
    char **strings = calloc(1, sizeof(char *));
    int nStrings = 0, nChars = 0;

    strings[0] = calloc(2, sizeof(char));

    while('\0' != *source)
    {
        if(*source == ' ')
        {
            nStrings++;
            nChars = 0;
            strings = realloc(strings, (1 + nStrings) * sizeof(char *));
            strings[nStrings] = calloc(2, sizeof(char));
            source++;
            continue;
        }

        strings[nStrings][nChars] = *source;
        strings[nStrings][nChars + 1] = '\0';

        nChars++;
        source++;
        strings[nStrings] = realloc(strings[nStrings], (2 + nChars) * sizeof(char));
    }

    return(strings);
}


Example usage:

char *source = "Hello Code Review";
char **words = strsplit(source);

printf("Second word is: %s", words[1]);


Output:


Code

Solution

Add some documentation to function.

While the name strsplit is pretty descriptive in itself, you should still add documentation to at least explain the argument and the return value.

For example, I was a little confused when I saw strsplit because I thought it should've taken another argument that was the delimiter.

Add some more comments to your code!

I had to rubberduck this code a couple times before I understood what was happening and where.

I think at least having a comment for what each variable was doing would help a lot.

Allocating and re-allocating memory is very inefficient. You should set one size of memory that will definitely fit the amount of memory needed.

For example, you could just allocate, for each output string, the size of the input string. However, this eats up a lot of memory for big strings.

You may have to come up with another algorithm for this.

You are sort-of reinventing the wheel here, and I don't think that that is your intent.

` already has a function char strtok(char str, const char *delim)`. However, this function only grabs tokens one at a time, so you'd have to take these steps:

  • Grab a token



  • Put it in the string array



  • Go to 1



Is roughly what you'd have to do.

I wrote out an implementation in C. If you'd like to see it, then leave a comment.

Context

StackExchange Code Review Q#95910, answer score: 5

Revisions (0)

No revisions yet.