patterncMinor
File of words to a linked-list of words
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.