4
\$\begingroup\$

The tokenise function below splits a given string at the indicated delimiters. As with strtok, it modifies the string by adding '\0's at the end of each token.

A pointer to the start of each token string is placed into the output array.

#include <string.h>

/* 
 * Wrapper around strtok
 * 
 * Splits a string at characters in "delimiters", writing each token to "output".
 * Delimiter characters in the source are overwritten with '\0'.
 * Stops tokenising if "output_size" is reached.
 * 
 * Returns 0 if "source", "delimiters", or "output" are NULL.
 * Returns 0 if "output_size" is 0.
 *
 * "source" must NOT be a string literal (modifying one is undefined behaviour).
 * "output" must be at least "output_size" long.
 * 
 * Example usage: 
 * char[] src = "a b c";
 * char* out[3]; 
 * size_t n = tokenise(src, " ", out, 3);
 * 
 */
size_t tokenise(char* source, char const* delimiters, char** output, size_t output_size)
{
    if (!source) return 0;
    if (!delimiters) return 0;
    if (!output) return 0;
    if (output_size < 1) return 0;

    char* const* start = output;
    char* const* end = output + output_size;

    *output = strtok(source, delimiters);

    while (*output && ++output != end)
        *output = strtok(NULL, delimiters);

    return output - start;
}


#include <stdio.h>
#include <stdbool.h>

void test(bool conditional)
{
    if (!conditional)
        printf("failed!\n");
}

void test_null_source_returns_zero()
{
    char* output[1];
    test(tokenise(NULL, " ", output, 1) == 0);
}

void test_null_delimiters_returns_zero()
{
    char source[] = " ";
    char* output[1];
    test(tokenise(source, NULL, output, 1) == 0);
    test(source[0] != '\0');
}

void test_null_output_returns_zero()
{
    char source[] = " ";
    test(tokenise(source, " ", NULL, 1) == 0);
    test(source[0] != '\0');
}

void test_zero_output_size_returns_zero()
{
    char source[] = " ";
    char* output[1];
    test(tokenise(source, " ", output, 0) == 0);
    test(source[0] != '\0');
}

void test_empty_source_returns_zero()
{
    char source[] = "";
    char* output[1];
    test(tokenise(source, " ", output, 1) == 0);
}

void test_source_without_delimiters_returns_one()
{
    char source[] = "sdlfkj";
    char* output[1];
    test(tokenise(source, " ", output, 1) == 1);
}

void test_source_with_only_delimiters_returns_zero()
{
    char source[] = "     ";
    char* output[1];
    test(tokenise(source, " ", output, 1) == 0);
    test(source[0] != '\0');
}

void test_empty_delimiters_returns_one()
{
    char source[] = "abc";
    char* output[1];
    test(tokenise(source, "", output, 1) == 1);
    test(strcmp(output[0], source) == 0);
}

void test_source_starts_with_delimiter()
{
    char source[] = "  abc";
    char* output[1];
    test(tokenise(source, " ", output, 1) == 1);
    test(source[0] != '\0');
    test(strcmp(output[0], "abc") == 0);
}

void test_source_ends_with_delimiter()
{
    char source[] = "a 123  ";
    char* output[2];
    test(tokenise(source, " ", output, 2) == 2);
    test(source[5] == '\0');
    test(source[6] != '\0');
    test(strcmp(output[0], "a") == 0);
    test(strcmp(output[1], "123") == 0);
}

void test_source_starts_and_ends_with_delimiter()
{
    char source[] = " a  bbb cccc dd ";
    char* output[4];
    test(tokenise(source, " ", output, 4) == 4);
    test(source[3] != '\0');
    test(strcmp(output[0], "a") == 0);
    test(strcmp(output[1], "bbb") == 0);
    test(strcmp(output[2], "cccc") == 0);
    test(strcmp(output[3], "dd") == 0);
}

void test_small_output_size_stops_tokenising()
{
    char source[] = "a b c d";
    char* output[2];
    test(tokenise(source, " ", output, 2) == 2);
    test(source[3] == '\0');
    test(source[5] != '\0');
    test(strcmp(output[0], "a") == 0);
    test(strcmp(output[1], "b") == 0);
}

void test_large_output_size_stops_at_end_of_source()
{
    char source[] = "a b c d";
    char* output[53];
    test(tokenise(source, " ", output, 53) == 4);
    test(strcmp(output[0], "a") == 0);
    test(strcmp(output[1], "b") == 0);
    test(strcmp(output[2], "c") == 0);
    test(strcmp(output[3], "d") == 0);
}

int main(void)
{
    test_null_source_returns_zero();
    test_null_delimiters_returns_zero();
    test_null_output_returns_zero();
    test_zero_output_size_returns_zero();
    test_empty_source_returns_zero();
    test_source_without_delimiters_returns_one();
    test_source_with_only_delimiters_returns_zero();
    test_empty_delimiters_returns_one();
    test_source_starts_with_delimiter();
    test_source_ends_with_delimiter();
    test_source_starts_and_ends_with_delimiter();
    test_small_output_size_stops_tokenising();
    test_large_output_size_stops_at_end_of_source();

    printf("done!\n");
}

Online version.

  • General feedback is welcome.
  • Are there any missing tests / edge cases?
  • Is there a better way to calculate the return size? (i.e. without the start variable`)
  • Some things (the contents of output and '\0's in the source) are tested as side-effects of the named test functions. Perhaps these should be separate named test cases?
  • I usually do C++ stuff. Is there anything more C-ish that I'm missing?
\$\endgroup\$
2
\$\begingroup\$

The tokenise() function is clear and succinct; I can't find anything to dislike. A couple of minor suggestions: wrap the declaration to a shorter line length, and consider changing the interface to terminate the output list with a null pointer (so that it's like argv).

It's great that you've included the unit tests for review. I love unit tests!

I recommend using an established test framework for the tests. It's fine to use a C++ unit-test framework for this, if we compile separately using extern "C" (as in this answer of mine). The big advantage you'll get is better diagnostics when tests fail: instead of just failed! in the output, you'll see which test failed, and the actual and expected values that caused the failure.

Yes, you can write your own EXPECT() macro (and I sometimes do, to make a simple test harness), but you'll find over time that it drifts towards the functionality of the existing unit-test libraries.

You might want to add a function/macro to test the output list of strings, to simplify this repetition:

test(strcmp(output[0], "a") == 0);
test(strcmp(output[1], "bbb") == 0);
test(strcmp(output[2], "cccc") == 0);
test(strcmp(output[3], "dd") == 0);

If you're writing the tests in C++, you get to use standard collections to make that side of things more readable:

test_strings_eq({"a", "bbb", "cccc", "dd"}, output);
\$\endgroup\$
2
\$\begingroup\$

Allow query

tokenise() obliges the calling code to know the maximum possible number of token prior to calling the function.

With judicious use of strspn(), strcspn(), the function could provide the return value of tokenise() without disturbing char* source past output_size.

With 2 passes:

size_t token_count = tokenise(source, delimiters, NULL, 0);
char** output = malloc(sizeof *ouput *token_count); 
tokenise(source, delimiters, output, token_count);

Quiet pedantic sign-ness change

output - start results in a signed integer type ptrdiff_t. OP's code silently converts to an unsigned type size_t. To quiet a picky compiler setting, apply a cast.

warning: conversion to 'size_t {aka long unsigned int}' from 'long int' may change the sign of the result [-Wsign-conversion]

//return output - start;
return (size_t) (output - start);

Coding error in comment

//char[] src = "a b c";
char src[] = "a b c";
\$\endgroup\$
0
\$\begingroup\$

LGTM.

A nitpick: testing for parameters being not-NULL is not enough. They may still being invalid (e.g. pointing to illegal places, or source being read-only). For true safety, consider catching SIGSEGV.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.