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

File of words to a linked-list of words

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

Problem

I'd like to get a review for the function build_list() which reads a file containing words (separated by spaces) and returns a linked-list of words:

#include 
#include 
#include 

typedef struct ListNode {
    char* word;
    struct ListNode *next;
} ListNode;

typedef struct List {
    ListNode *head;
    ListNode *tail;
} List;

List* build_list(char* fileName)
{
    FILE* file = fopen(fileName, "r");
    assert(file != NULL);

    List* list = malloc(sizeof(List));
    assert(list != NULL);
    list->head = NULL;
    list->tail = NULL;

    char c;
    int word_size = 1;
    char* word = malloc(word_size);
    int word_count = 0;

    while ((c = fgetc(file)) != EOF)
    {
        assert(!ferror(file));

        while (c != ' ' && c != EOF)
        {
            if (word_count == word_size)
            {
                word_size += 1;
                word = realloc(word, word_size);
                assert(word != NULL);
            }

            word[word_count++] = c;

            c = fgetc(file);
            assert(!ferror(file));
        }

        word = realloc(word, word_size + 1);
        word[word_size] = '\0';

        ListNode *node = malloc(sizeof(ListNode));
        assert(node != NULL);
        node->next = NULL;
        node->word = word;

        if (list->head == NULL)
        {
            list->head = node;
            list->tail = node;
        }
        else
        {
            list->tail->next = node;
            list->tail = node;
        }

        if (c == EOF)
            break;

        word_count = 0;
        word_size = 1;
        word = malloc(word_size);

    }

    free(word);

    assert(!ferror(file));

    return list;
}

Solution

assert() isn't meant for runtime errors like an invalid filename. It's meant to aid in debugging by catching logical errors. Essentially, it says, "There should be no possible way this statement can be false. If it is false, something about this program is invalid, so I need to blow up."

A good use of assert() here would be:

List* build_list(char* fileName)
{
    assert(fileName && fileName[0]);
    // Alternately (more verbose, but maybe more clear):
    assert(fileName != NULL && fileName[0] != '\0');
    // Another alternative, which gives a helpful message
    // if it fails
    assert(fileName && fileName[0]
        && "build_list(char* fileName) was given a null or empty fileName");


This way, if you write a function somewhere else that passes a null filename into build_list, you'll get notified of the programming error.

A runtime error such as a missing file is better handled by returning some kind of error. You could return NULL on error, or you could take a List* (or List**) as an argument and return an integer result (0 for success is typical).

Code Snippets

List* build_list(char* fileName)
{
    assert(fileName && fileName[0]);
    // Alternately (more verbose, but maybe more clear):
    assert(fileName != NULL && fileName[0] != '\0');
    // Another alternative, which gives a helpful message
    // if it fails
    assert(fileName && fileName[0]
        && "build_list(char* fileName) was given a null or empty fileName");

Context

StackExchange Code Review Q#101027, answer score: 5

Revisions (0)

No revisions yet.