5
\$\begingroup\$

(See the previous iteration.)

What does this program do?

This program gives you a command line interface for managing your personal telephone book. The program supports three actions:

  1. Adding a telephone book entry.
  2. Removing entries by their IDs.
  3. Listing the entries.

Synopsis

For adding an entry, type: ./TelephoneBook -a Dijkstra Edsger 12321 or ./TelephoneBook --add Dijkstra Edsger 12321

For removing entries by their IDs (which will be unique), type: ./TelephoneBook -r ID1 ID2 ... IDn or ./TelephoneBook --remove ID1 ID2 ... IDn

For listing all entries, invoke the program with no arguments.

For approximate match for listing entries, type: ./TelephoneBook LAST_NAME, ./TelephoneBook LAST_NAME FIRST_NAME, ./TelephoneBook * FIRST_NAME.

The output may look like:

----------+------------+------------------+---
Last name | First name | Telephone number | ID
----------+------------+------------------+---
Bro       | Vector     | 11               | 1 
Efremov   | Rodion     | 8055             | 2 
Ervasti   | Timo       | 1053             | 3 
----------+------------+------------------+---
Ervasti   | Violetta   | 3311376          | 4 
Funk      | Funky      | 12321            | 5 
Minogue   | Kylie      | 4401             | 6 
----------+------------+------------------+---
Ryazanov  | Viktor     | 3454             | 7

Finally, for the help message, type: ./TelephoneBook -h | --help.

The actual file holding the data is put into the user's home directory, and is named .telephone_book.

Requirements

One explicit requirement for my program is that it is pure ANSI C for the sake of portability.

What's new

Now I have included all the points made by chux.

  1. The linked list structure holding the telephone book entries is now a singly-linked list.
  2. If reading an entry from the file fails, the program prints an error message and exits instead of silently ignoring the error.
  3. Removed redundant comments from .c files.
  4. Checking for even more errors.
  5. Getting rid with sizeof(char) since it is guaranteed to always be 1.
  6. Better token format strings.

Code

Now I have this:

telephone_book.h 
#ifndef TELEPHONE_BOOK_H
#define TELEPHONE_BOOK_H

/*******************************************************************************
* This structure holds a single telephone book record.                         *
*******************************************************************************/
typedef struct {
    char* first_name;
    char* last_name;
    char* telephone_number;
    int id;
} telephone_book_record;

/*******************************************************************************
* This structure defines a linked list node for the telephone book record      *
* list.                                                                        *
*******************************************************************************/
typedef struct telephone_book_record_list_node {
    telephone_book_record* record;
    struct telephone_book_record_list_node* next;
} telephone_book_record_list_node;

/*******************************************************************************
* This structure holds a doubly-linked list of telephone book records.         *
*******************************************************************************/
typedef struct {
    struct telephone_book_record_list_node* head;
    struct telephone_book_record_list_node* tail;
    int size;
} telephone_book_record_list;




/*******************************************************************************
* Returns the length of the telephone book record list.                        *
*******************************************************************************/
int telephone_book_record_list_size(telephone_book_record_list* list);

/*******************************************************************************
* Allocates and initializes a new telephone book record.                       *
* ---                                                                          *
* Returns a new telephone book record or NULL if something goes wrong.         *
*******************************************************************************/
telephone_book_record* telephone_book_record_alloc(const char* last_name,
                                                   const char* first_name,
                                                   const char* phone_number,
                                                   int id);

/*******************************************************************************
* Frees the memory occupied by the telephone book record: all existing fields  *
* and the actual record.                                                       *
*******************************************************************************/
void telephone_book_record_free(telephone_book_record* record);




/*******************************************************************************
* Allocates and initializes an empty telephone book record list.               *
* ---                                                                          *
* Returns a new empty telephone book record list or NULL if something goes     *
* wrong.                                                                       *
*******************************************************************************/
telephone_book_record_list* telephone_book_record_list_alloc();

/*******************************************************************************
* Appends the argument telephone book record to the tail of the argument       *
* telephone book record list.                                                  *
* ---                                                                          *
* Returns a zero value if the operation was successfull. A non-zero value is   *
* returned if something fails.                                                 *
*******************************************************************************/
int telephone_book_record_list_add_record(telephone_book_record_list* list,
                                          telephone_book_record* record);

/*******************************************************************************
* Removes and returns the telephone book record that has 'id' as its record ID.*
* ---                                                                          *
* Returns NULL if something fails or the list does not contain record with ID  *
* 'id'. Otherwise, a removed record is returned.                               *
*******************************************************************************/
telephone_book_record*
telephone_book_record_list_remove_entry(telephone_book_record_list* list,
                                        int id);

/*******************************************************************************
* Sorts the telephone records. The last name of each record is the primary     *
* sorting key, and the first name of each record is the secondary sorting key. *
* ---                                                                          *
* Returns zero on success, and a non-zero value if the sorting could not be    *
* completed.                                                                   *
*******************************************************************************/
int telephone_book_record_list_sort(telephone_book_record_list* list);

/*******************************************************************************
* Makes sure that each telephone book record has a unique ID.                  *
* ---                                                                          *
* Returns zero on success, and a non-zero value if something fails.            *
*******************************************************************************/
int telephone_book_record_list_fix_ids(telephone_book_record_list* list);

/*******************************************************************************
* Frees all the memory occupied by the argument telephone book record list.    *
*******************************************************************************/
void telephone_book_record_list_free(telephone_book_record_list* list);

#endif /* TELEPHONE_BOOK_H */
telephone_book.c 
#include "telephone_book.h"
#include "telephone_book_utils.h"
#include <stdlib.h>
#include <string.h>

#define MAX(a, b) ((a) > (b) ? (a) : (b))

/*******************************************************************************
* Documentation comments may be found in telephone_book.h                      *
*******************************************************************************/


int telephone_book_record_list_size(telephone_book_record_list* list)
{

    return list ? list->size : -1;
}

static telephone_book_record_list_node*
telephone_book_record_list_node_alloc(telephone_book_record* record)
{
    telephone_book_record_list_node* node;

    if (!record)
    {
        return NULL;
    }

    node = malloc(sizeof *node);

    if (!node)
    {
        return NULL;
    }

    node->record = record;
    node->next = NULL;

    return node;
}

telephone_book_record* telephone_book_record_alloc(const char* last_name,
                                                   const char* first_name,
                                                   const char* phone_number,
                                                   int id)
{
    telephone_book_record* record = malloc(sizeof *record);

    if (!record)
    {
        return NULL;
    }

    record->last_name        = malloc(strlen(last_name) + 1);
    record->first_name       = malloc(strlen(first_name) + 1);
    record->telephone_number = malloc(strlen(phone_number) + 1);
    record->id = id;

    strcpy(record->last_name, last_name);
    strcpy(record->first_name, first_name);
    strcpy(record->telephone_number, phone_number);

    return record;
}

void telephone_book_record_free(telephone_book_record* record)
{
    if (!record)
    {
        return;
    }

    free(record->first_name);
    free(record->last_name);
    free(record->telephone_number);
    free(record);
}

telephone_book_record_list* telephone_book_record_list_alloc()
{
    telephone_book_record_list* record_list = malloc(sizeof *record_list);

    if (!record_list)
    {
        return NULL;
    }

    record_list->head = NULL;
    record_list->tail = NULL;
    record_list->size = 0;
    return record_list;
}

int telephone_book_record_list_add_record(telephone_book_record_list* list,
                                          telephone_book_record* record)
{
    telephone_book_record_list_node* new_node;

    if (!list || !record)
    {
        return 1;
    }

    new_node = telephone_book_record_list_node_alloc(record);

    if (!new_node)
    {
        return 1;
    }

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

    list->tail = new_node;
    list->size++;
    return 0;
}

telephone_book_record*
telephone_book_record_list_remove_entry(telephone_book_record_list* list,
                                        int id)
{
    telephone_book_record_list_node* previous_node;
    telephone_book_record_list_node* current_node;
    telephone_book_record_list_node* next_node;
    telephone_book_record* removed_record;

    if (!list)
    {
        return NULL;
    }

    previous_node = NULL;
    current_node = list->head;

    while (current_node)
    {
        next_node = current_node->next;

        /* Since the program makes sure that all IDs are unique, we can */
        /* return as soon as we remove the first match. */
        if (current_node->record->id == id)
        {
            if (previous_node)
            {
                previous_node->next = current_node->next;
            }
            else
            {
                list->head = current_node->next;
            }

            if (!current_node->next)
            {
                list->tail = previous_node;
            }

            removed_record = current_node->record;
            free(current_node);
            return removed_record;
        }

        previous_node = current_node;
        current_node = next_node;
    }

    return NULL;
}

static int record_cmp(const void* pa, const void* pb)
{
    int c;

    telephone_book_record_list_node* a =
    *(telephone_book_record_list_node *const *) pa;

    telephone_book_record_list_node* b =
    *(telephone_book_record_list_node *const *) pb;

    c = strcmp(a->record->last_name, b->record->last_name);

    if (c)
    {
        return c;
    }

    return strcmp(a->record->first_name, b->record->first_name);
}

int telephone_book_record_list_sort(telephone_book_record_list* list)
{
    telephone_book_record_list_node** array;
    telephone_book_record_list_node* current_node;

    int list_length;
    int index;

    if (!list)
    {
        return 1;
    }

    list_length = telephone_book_record_list_size(list);

    if (list_length == 0)
    {
        /* Nothing to sort. */
        return 0;
    }

    array = malloc(list_length * sizeof *array);

    if (!array)
    {
        return 1;
    }

    current_node = list->head;

    for (index = 0;
         index < list_length;
         index++, current_node = current_node->next)
    {
        array[index] = current_node;
    }

    qsort(array, list_length, sizeof *array, record_cmp);

    /* Relink the nodes: */
    list->head = array[0];
    list->tail = array[list_length - 1];

    list->tail->next = NULL;

    for (index = 0; index < list_length - 1; ++index)
    {
        array[index]->next = array[index + 1];
    }

    /* Freeing memory! */
    free(array);
    return 0;
}

int telephone_book_record_list_fix_ids(telephone_book_record_list* list)
{
    int id;
    telephone_book_record_list_node* current_node;

    if (!list)
    {
        return 1;
    }

    id = 0;
    current_node = list->head;

    while (current_node)
    {
        current_node->record->id = ++id;
        current_node = current_node->next;
    }

    return 0;
}

void telephone_book_record_list_free(telephone_book_record_list* list)
{
    telephone_book_record_list_node* current_node;
    telephone_book_record_list_node* next_node;

    if (!list)
    {
        return;
    }

    current_node = list->head;

    while (current_node)
    {
        next_node = current_node->next;
        telephone_book_record_free(current_node->record);
        free(current_node);
        current_node = next_node;
    }

    free(list);
}
telephone_book_io.h 
#ifndef TELEPHONE_BOOK_IO_H
#define TELEPHONE_BOOK_IO_H

#include "telephone_book.h"
#include <stdio.h>

/*******************************************************************************
* Reconstructs the telephone book record list from a file pointed to by the    *
* argument file handle.                                                        *
* ---                                                                          *
* Returns the record list on success, and NULL on failure.                     *
*******************************************************************************/
telephone_book_record_list* telephone_book_record_list_read_from_file(FILE* f);

/*******************************************************************************
* Writes the entire contents of the telephone record list to a specified file  *
* handle.                                                                      *
* ---                                                                          *
* Returns zero on success, and a non-zero value if something fails.            *
*******************************************************************************/
int telephone_book_record_list_write_to_file(telephone_book_record_list* list,
                                             FILE* f);


#endif /* TELEPHONE_BOOK_IO_H */
telephone_book_io.c 
#include "telephone_book_io.h"
#include <stdio.h>

#define TOKEN_SCAN_FORMAT "%64s"
#define MAX_RECORD_TOKEN_LENGTH 65

/*******************************************************************************
* Documentation comments may be found in telephone_book_io.h                   *
*******************************************************************************/


telephone_book_record_list* telephone_book_record_list_read_from_file(FILE* f)
{
    telephone_book_record_list* record_list;
    telephone_book_record* current_record;

    char last_name_token   [MAX_RECORD_TOKEN_LENGTH];
    char first_name_token  [MAX_RECORD_TOKEN_LENGTH];
    char phone_number_token[MAX_RECORD_TOKEN_LENGTH];
    int  id_holder;
    int read_result;

    if (!f)
    {
        return NULL;
    }

    record_list = telephone_book_record_list_alloc();

    if (!record_list)
    {
        return NULL;
    }

    while (!feof(f) && !ferror(f))
    {
        read_result = fscanf(f,
                             TOKEN_SCAN_FORMAT
                             TOKEN_SCAN_FORMAT
                             TOKEN_SCAN_FORMAT
                             "%d\n",
                             last_name_token,
                             first_name_token,
                             phone_number_token,
                             &id_holder);

        if (read_result == 4)
        {
            current_record = telephone_book_record_alloc(last_name_token,
                                                         first_name_token,
                                                         phone_number_token,
                                                         id_holder);
            if (!current_record)
            {
                telephone_book_record_list_free(record_list);
                return NULL;
            }

            if (telephone_book_record_list_add_record(record_list,
                                                      current_record))
            {
                telephone_book_record_list_free(record_list);
                telephone_book_record_free(current_record);
                return NULL;
            }
        }
        else
        {
            fclose(f);
            telephone_book_record_list_free(record_list);
            return NULL;
        }
    }

    return record_list;
}

int telephone_book_record_list_write_to_file(telephone_book_record_list* list,
                                             FILE* f)
{
    telephone_book_record_list_node* current_node;

    if (!list || !f)
    {
        return 1;
    }

    if (list->size == 0)
    {
        return 0;
    }

    current_node = list->head;

    while (current_node)
    {
        fprintf(f,
                "%s %s %s %d\n",
                current_node->record->last_name,
                current_node->record->first_name,
                current_node->record->telephone_number,
                current_node->record->id);

        current_node = current_node->next;
    }

    return 0;
}
telephone_book_utils.h 
#ifndef TELEPHONE_BOOK_UTILS_H
#define TELEPHONE_BOOK_UTILS_H

#include <stdio.h>
#include "telephone_book.h"

#ifdef _WIN32
#define PATH_SEPARATOR '\\'
#else
#define PATH_SEPARATOR '/'
#endif

/*******************************************************************************
* This structures holds the string required for neat result output.            *
*******************************************************************************/
typedef struct {
    char* title_string;
    char* separator_string;
    char* record_format_string;
} output_table_strings;




/*******************************************************************************
* Returns a C string representing the full path to the telephone book record   *
* file.                                                                        *
*******************************************************************************/
char* get_telephone_record_book_file_path();

/*******************************************************************************
* Creates and returns all format strings for printing the record list.         *
*******************************************************************************/
output_table_strings*
output_table_strings_create(telephone_book_record_list* list);

/*******************************************************************************
* Creates and returns a structure containing all format strings necessary for  *
* printing the telephone book record list.                                     *
*******************************************************************************/
void output_table_strings_free(output_table_strings* output_table_strs);

/*******************************************************************************
* Returns the format string for nifty printing the removed records.            *
*******************************************************************************/
char* get_removed_record_output_format_string(telephone_book_record_list* list);

#endif /* TELEPHONE_BOOK_UTILS_H */
telephone_book_utils.c 
#include "telephone_book_utils.h"
#include "telephone_book.h"
#include <stdlib.h>
#include <string.h>

#define MAX(a, b) ((a) > (b) ? (a) : (b))

const char* TELEPHONE_RECORD_BOOK_FILE_NAME = ".telephone_book";

static const char* TITLE_LAST_NAME          = "Last name";
static const char* TITLE_FIRST_NAME         = "First name";
static const char* TITLE_TELEPHONE_NUMBER   = "Telephone number";
static const char* TITLE_CONTACT_ID         = "ID";

static const size_t ID_HOLDER_STRING_CAPACITY = 40;
static const size_t FORMAT_STRING_CAPACITY    = 100;

/*******************************************************************************
* Documentation comments may be found in telephone_book_utils.h                *
*******************************************************************************/


char* get_telephone_record_book_file_path()
{
    char* home_directory;
    char* telephone_record_book_file_path;
    size_t home_directory_name_length;

    home_directory = getenv("HOME");

    if (!home_directory)
    {
        return NULL;
    }

    home_directory_name_length = strlen(home_directory);

    /* Allocate the space for the entire */
    telephone_record_book_file_path =
    malloc(home_directory_name_length
           +
           2
           +
           strlen(TELEPHONE_RECORD_BOOK_FILE_NAME));

    if (!telephone_record_book_file_path)
    {
        return NULL;
    }

    strcpy(telephone_record_book_file_path, home_directory);

    telephone_record_book_file_path[home_directory_name_length] =
        PATH_SEPARATOR;

    strcpy(&telephone_record_book_file_path[home_directory_name_length + 1],
           TELEPHONE_RECORD_BOOK_FILE_NAME);

    return telephone_record_book_file_path;
}

static char* write_separator(char* str, char c, size_t n)
{
    memset(str, c, n);
    return str + n;
}

char* load_separator_string(size_t max_last_name_token_length,
                            size_t max_first_name_token_length,
                            size_t max_telephone_number_token_length,
                            size_t max_telephone_contact_id_length)
{
    char* save_separator_string;

    /* The separator string is composed of 4 horizontal bars: one bar for the
     * last name, one for first name, one for phone number, and one for the ID
     * (in that order). The magic constants 1 and 2, are the additional padding
     * so that for each attribute (column) there is a space before and after the
     * column title. 'max_first_name_token_length + 2' means that the longest
     * first name (or column) title is no longer than 
     * 'max_first_name_token_length', and it will be preceded and followed by 
     * one space.
     *
     * The magic constant 4, counts 3 bars between the columns and a final 
     * '\0' terminator.
     */
    char* separator_string =
    malloc((max_last_name_token_length + 1) +
           (max_first_name_token_length + 2) +
           (max_telephone_number_token_length + 2) +
           (max_telephone_contact_id_length + 1) + 4);

    if (!separator_string)
    {
        return NULL;
    }

    save_separator_string = separator_string;
    separator_string = write_separator(separator_string,
                                       '-',
                                       max_last_name_token_length + 1);

    separator_string = write_separator(separator_string, '+', 1);
    separator_string = write_separator(separator_string,
                                       '-',
                                       max_first_name_token_length + 2);

    separator_string = write_separator(separator_string, '+', 1);
    separator_string = write_separator(separator_string,
                                       '-',
                                       max_telephone_number_token_length + 2);

    separator_string = write_separator(separator_string, '+', 1);
    separator_string = write_separator(separator_string,
                                       '-',
                                       max_telephone_contact_id_length + 1);

    write_separator(separator_string, '\0', 1);
    return save_separator_string;
}

output_table_strings*
output_table_strings_create(telephone_book_record_list* list)
{
    size_t max_last_name_token_length        = strlen(TITLE_LAST_NAME);
    size_t max_first_name_token_length       = strlen(TITLE_FIRST_NAME);
    size_t max_telephone_number_token_length = strlen(TITLE_TELEPHONE_NUMBER);
    size_t max_telephone_contact_id_length   = strlen(TITLE_CONTACT_ID);

    size_t last_name_token_length;
    size_t first_name_token_length;
    size_t telephone_number_token_length;
    size_t telephone_contact_id_length;

    output_table_strings* output_table;

    /* The format string used to output the actual telephone book records. */
    char* record_format_string;

    /* The title string. */
    char* title_string;

    /* The separating horizontal bar. */
    char* separator_string;
    char* id_holder_string;

    telephone_book_record_list_node* current_node;
    telephone_book_record* current_record;

    if (!list)
    {
        return NULL;
    }

    /* ALLOCATED: output_table */
    output_table = malloc(sizeof *output_table);

    if (!output_table)
    {
        return NULL;
    }

    /* ALLOCATED: output_table, id_holder_string */
    id_holder_string = malloc(ID_HOLDER_STRING_CAPACITY);

    if (!id_holder_string)
    {
        free(output_table);
        return NULL;
    }

    current_node = list->head;

    while (current_node)
    {
        current_record = current_node->record;

        last_name_token_length  = strlen(current_record->last_name);
        first_name_token_length = strlen(current_record->first_name);
        telephone_number_token_length =
        strlen(current_record->telephone_number);

        sprintf(id_holder_string, "%d", current_record->id);
        telephone_contact_id_length = strlen(id_holder_string);

        max_last_name_token_length = MAX(max_last_name_token_length,
                                         last_name_token_length);

        max_first_name_token_length = MAX(max_first_name_token_length,
                                          first_name_token_length);

        max_telephone_number_token_length =
            MAX(max_telephone_number_token_length,
                telephone_number_token_length);

        max_telephone_contact_id_length = MAX(max_telephone_contact_id_length,
                                              telephone_contact_id_length);

        current_node = current_node->next;
    }

    /* ALLOCATED: output_table */
    free(id_holder_string);

    /* ALLOCATED: output_table, record_format_string */
    record_format_string = malloc(FORMAT_STRING_CAPACITY);

    if (!record_format_string)
    {
        free(output_table);
        return NULL;
    }

    /* This is really ad hoc, yet we want to keep the listing code as simple 
       as possible, so we opt to do the format magic in this routine. */
    sprintf(record_format_string,
            "%%-%zus | %%-%zus | %%-%zus | %%-%zuzu\n",
            max_last_name_token_length,
            max_first_name_token_length,
            max_telephone_number_token_length,
            max_telephone_contact_id_length);

    /* @ALLOC: output_table, record_format_string, title_string */
    title_string = malloc((max_last_name_token_length + 1) +
                          (max_first_name_token_length + 2) +
                          (max_telephone_number_token_length + 2) +
                          (max_telephone_contact_id_length + 1) + 4);

    if (!title_string)
    {
        free(output_table);
        free(record_format_string);
        return NULL;
    }

    sprintf(title_string,
            "%-*s | %-*s | %-*s | %-*s",
            (int) max_last_name_token_length,
            TITLE_LAST_NAME,
            (int) max_first_name_token_length,
            TITLE_FIRST_NAME,
            (int) max_telephone_number_token_length,
            TITLE_TELEPHONE_NUMBER,
            (int) max_telephone_contact_id_length,
            TITLE_CONTACT_ID);

    /* @ALLOC: output_table, record_format_string, title_string, 
               separator_string*/
    separator_string =
        load_separator_string(max_last_name_token_length,
                              max_first_name_token_length,
                              max_telephone_number_token_length,
                              max_telephone_contact_id_length);

    if (!separator_string)
    {
        free(output_table);
        free(record_format_string);
        free(title_string);
        return NULL;
    }

    output_table->title_string = title_string;
    output_table->separator_string = separator_string;
    output_table->record_format_string = record_format_string;

    return output_table;
}

void output_table_strings_free(output_table_strings* output_table_strs)
{
    if (!output_table_strs)
    {
        return;
    }

    free(output_table_strs->record_format_string);
    free(output_table_strs->separator_string);
    free(output_table_strs->title_string);
    free(output_table_strs);
}

char* get_removed_record_output_format_string(telephone_book_record_list* list)
{
    size_t max_last_name_token_length        = 0;
    size_t max_first_name_token_length       = 0;
    size_t max_telephone_number_token_length = 0;
    size_t max_telephone_contact_id_length   = 0;

    size_t last_name_token_length;
    size_t first_name_token_length;
    size_t telephone_number_token_length;
    size_t telephone_contact_id_length;

    /* ALLOCATED: format_string */
    char* format_string = malloc(FORMAT_STRING_CAPACITY);
    char* id_holder_string;

    telephone_book_record_list_node* current_node;
    telephone_book_record* current_record;

    if (!format_string)
    {
        return NULL;
    }

    /* ALLOCATED: format_string, id_holder_string */
    id_holder_string = malloc(ID_HOLDER_STRING_CAPACITY);

    if (!id_holder_string)
    {
        return NULL;
    }

    current_node = list->head;

    while (current_node)
    {
        current_record = current_node->record;

        last_name_token_length  = strlen(current_record->last_name);
        first_name_token_length = strlen(current_record->first_name);
        telephone_number_token_length =
                                  strlen(current_record->telephone_number);

        sprintf(id_holder_string, "%d", current_record->id);
        telephone_contact_id_length = strlen(id_holder_string);

        max_last_name_token_length = MAX(max_last_name_token_length,
                                         last_name_token_length);

        max_first_name_token_length = MAX(max_first_name_token_length,
                                          first_name_token_length);

        max_telephone_number_token_length =
        MAX(max_telephone_number_token_length,
            telephone_number_token_length);

        max_telephone_contact_id_length = MAX(max_telephone_contact_id_length,
                                              telephone_contact_id_length);
        current_node = current_node->next;
    }



    sprintf(format_string,
            "%%-%zus %%-%zus %%-%zus %%-%zuzu\n",
            max_last_name_token_length,
            max_first_name_token_length,
            max_telephone_number_token_length,
            max_telephone_contact_id_length);

    /* ALLOCATED: format_string */
    free(id_holder_string);
    return format_string;
}
main.c 
#include "telephone_book.h"
#include "telephone_book_io.h"
#include "telephone_book_utils.h"
#include <ctype.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define ERROR "[ERROR] "
#define WARNING "[WARNING] "
#define INFO "[INFO] "

static const char* OPTION_ADD_SHORT = "-a";
static const char* OPTION_ADD_LONG  = "--add";

static const char* OPTION_REMOVE_SHORT = "-r";
static const char* OPTION_REMOVE_LONG  = "--remove";

static const char* OPTION_HELP_SHORT = "-h";
static const char* OPTION_HELP_LONG  = "--help";

static const size_t RECORDS_PER_BLOCK = 3;

/*******************************************************************************
* Returns the smallest of 'a', 'b', and 'c'.                                   *
*******************************************************************************/
static size_t min3(size_t a, size_t b, size_t c)
{
    if (a < b)
    {
        if (c < a)
        {
            return c;
        }

        return a;
    }

    /* b <= a */
    if (c < b)
    {
        return c;
    }

    return b;
}

/*******************************************************************************
* Prints the help message to the standard output.                              *
*******************************************************************************/
static void print_help(char* executable_name)
{
    char path_separator = PATH_SEPARATOR;
    int executable_name_length = (int) strlen(executable_name);
    int i;

    /* Doing my own basename for the sake of portability. */
    for (i = executable_name_length - 1; i >= 0; --i)
    {
        if (executable_name[i] == path_separator)
        {
            executable_name = &executable_name[i + 1];
            break;
        }
    }

    printf("Usage: %s -a    LAST FIRST NUMBER\n", executable_name);
    printf("       %s --add LAST FIRST NUMBER\n", executable_name);
    puts("");
    printf("       %s -r       ID1 ID2 ... IDn\n", executable_name);
    printf("       %s --remove ID1 ID2 ... IDn\n", executable_name);
    puts("");
    printf("(1)    %s\n",                      executable_name);
    printf("(2)    %s LAST_EXPR\n",            executable_name);
    printf("(3)    %s - FIRST_EXPR\n",         executable_name);
    printf("(4)    %s LAST_EXPR FIRST_EXPR\n", executable_name);
    puts("");
    puts("Where: -a or --add for adding one new book entry.");
    puts("       -r or --remove for removing book entries by their IDs.");
    puts("");
    puts("(1) List all book entries in order.");
    puts("(2) Match by last name and list the closest book entries.");
    puts("(3) Match by first name and list the closest book entries.");
    puts("(4) Match by both last and first names and list the closest book "
         "entries");
}

/*******************************************************************************
* Implements the Levenshtein distance algorithm.                               *
*******************************************************************************/
static size_t edit_distance(char* word1,
                            char* word2,
                            size_t length1,
                            size_t length2)
{
    int cost;

    if (length1 == 0)
    {
        return length2;
    }

    if (length2 == 0)
    {
        return length1;
    }

    cost = tolower(word1[length1 - 1]) ==
           tolower(word2[length2 - 1]) ? 0 : 1;

    return min3(edit_distance(word1, word2, length1, length2 - 1) + 1,
                edit_distance(word1, word2, length1 - 1, length2) + 1,
                edit_distance(word1, word2, length1 - 1, length2 - 1) + cost);
}

/*******************************************************************************
* Computes the Levenshtein disance between words 'word1' and 'word2'.          *
*******************************************************************************/
static size_t compute_edit_distance(char* word1, char* word2)
{
    return edit_distance(word1, word2, strlen(word1), strlen(word2));
}

/*******************************************************************************
* Implements listing the telephone book records.                               *
*******************************************************************************/
int command_list_telephone_book_records_impl(
                        telephone_book_record_list* record_list,
                        char* last_name,
                        char* first_name)
{
    size_t best_tentative_distance = 1000 * 1000 * 1000;
    size_t temp_distance;
    size_t i;
    output_table_strings* output_strings;
    telephone_book_record* record;
    telephone_book_record_list_node* current_node;
    telephone_book_record_list* best_record_list =
        telephone_book_record_list_alloc();

    /* ALLOCATED: best_record_list */
    if (!best_record_list)
    {
        fputs(ERROR "Cannot allocate the best record list.\n", stderr);
        return EXIT_FAILURE;
    }

    current_node = record_list->head;

    while (current_node)
    {
        temp_distance =
        (last_name ?
            compute_edit_distance(last_name,
                                  current_node->record->last_name) : 0) +
        (first_name ?
            compute_edit_distance(first_name,
                                  current_node->record->first_name) : 0);

        if (best_tentative_distance > temp_distance)
        {
            /* 'temp_distance' improves the best known edit distance,     */
            /* clear the current best list and append the current record: */
            telephone_book_record_list_free(best_record_list);
            best_record_list = telephone_book_record_list_alloc();

            if (!best_record_list)
            {
                fputs(ERROR "Cannot allocate new best record list.\n", stderr);
                return EXIT_FAILURE;
            }

            record = telephone_book_record_alloc(
                            current_node->record->last_name,
                            current_node->record->first_name,
                            current_node->record->telephone_number,
                            current_node->record->id);

            if (!record)
            {
                fputs(ERROR "Cannot allocate a copy record.\n", stderr);
                telephone_book_record_list_free(best_record_list);
                return EXIT_FAILURE;
            }

            if (telephone_book_record_list_add_record(best_record_list, record))
            {
                fputs(ERROR "Cannot add a new record to the best list.\n",
                      stderr);
                telephone_book_record_list_free(best_record_list);
                telephone_book_record_free(record);
                return EXIT_FAILURE;
            }

            best_tentative_distance = temp_distance;
        }
        else if (best_tentative_distance == temp_distance)
        {
            /* Append the current record to the best list: */
            record = telephone_book_record_alloc(
                            current_node->record->last_name,
                            current_node->record->first_name,
                            current_node->record->telephone_number,
                            current_node->record->id);

            if (!record)
            {
                fputs(ERROR "Cannot allocate a copy record.\n", stderr);
                telephone_book_record_list_free(best_record_list);
                return EXIT_FAILURE;
            }


            if (telephone_book_record_list_add_record(best_record_list, record))
            {
                fputs(ERROR "Cannot add a new record to the best list.\n",
                      stderr);
                telephone_book_record_list_free(best_record_list);
                telephone_book_record_free(record);
                return EXIT_FAILURE;
            }
        }

        current_node = current_node->next;
    }

    output_strings = output_table_strings_create(best_record_list);

    if (!output_strings)
    {
        telephone_book_record_list_free(best_record_list);
        return EXIT_FAILURE;
    }

    puts(output_strings->separator_string);
    puts(output_strings->title_string);
    puts(output_strings->separator_string);

    current_node = best_record_list->head;
    i = 0;

    while (current_node)
    {
        printf(output_strings->record_format_string,
               current_node->record->last_name,
               current_node->record->first_name,
               current_node->record->telephone_number,
               current_node->record->id);

        current_node = current_node->next;
        ++i;

        if (current_node && i % RECORDS_PER_BLOCK == 0)
        {
            puts(output_strings->separator_string);
        }
    }

    output_table_strings_free(output_strings);
    telephone_book_record_list_free(best_record_list);
    return EXIT_SUCCESS;
}

/*******************************************************************************
* Handles the command for listing the records.                                 *
*******************************************************************************/
static int command_list_telephone_book_records(int argc, char* argv[])
{
    char* file_name;
    FILE* f;
    telephone_book_record_list* record_list;
    char* last_name;
    char* first_name;

    /* ALLOCATED: file_name */
    file_name = get_telephone_record_book_file_path();

    if (!file_name)
    {
        fputs(ERROR
              "Cannot allocate memory for the telephone book file name.\n",
              stderr);
        return EXIT_FAILURE;
    }

    f = fopen(get_telephone_record_book_file_path(), "r");

    if (!f)
    {
        fprintf(stderr,
                ERROR "Cannot open the record book file '%s'.\n",
                file_name);

        free(file_name);
        return EXIT_FAILURE;
    }

    /* ALLOCATED: file_name, record_list */
    record_list = telephone_book_record_list_read_from_file(f);
    fclose(f);

    if (!record_list)
    {
        fputs(ERROR "Cannot read the record book file.\n", stderr);
        free(file_name);
        return EXIT_FAILURE;
    }

    /* If fails, silently ignore: */
    telephone_book_record_list_sort(record_list);
    /* Does not ask for resources, should be OK: */
    telephone_book_record_list_fix_ids(record_list);

    /* "w" means overwrite the file. */
    f = fopen(file_name, "w");

    if (f)
    {
        /* Write the file back. It will update the order of the records and */
        /* fix the record IDs, if needed. */
        telephone_book_record_list_write_to_file(record_list, f);
        fclose(f);
    }

    last_name  = argc >= 2 ? argv[1] : NULL;
    first_name = argc >= 3 ? argv[2] : NULL;

    if (argc > 1 && strcmp(argv[1], "-") == 0)
    {
        /* Match all last names: */
        last_name = NULL;
    }

    /* ALLOCATED: record_list */
    free(file_name);

    return command_list_telephone_book_records_impl(record_list,
                                                    last_name,
                                                    first_name);
}

/*******************************************************************************
* Handles the command for adding a new record.                                 *
*******************************************************************************/
static int command_add_record(int argc, char* argv[])
{
    char* file_name;
    FILE* f;
    telephone_book_record_list* record_list;
    telephone_book_record* record;

    if (argc != 5)
    {
        print_help(argv[0]);
        return EXIT_FAILURE;
    }

    /* ALLOCATED: file_name */
    file_name = get_telephone_record_book_file_path();

    if (!file_name)
    {
        fputs(ERROR
              "Cannot allocate memory for the telephone book file name.\n",
              stderr);
        return EXIT_FAILURE;
    }

    f = fopen(file_name, "r");

    if (!f)
    {
        fprintf(stderr, ERROR "Cannot open the record book file '%s'.\n",
                file_name);

        free(file_name);
        return EXIT_FAILURE;
    }

    /* ALLOCATED: file_name, record_list */
    record_list = telephone_book_record_list_read_from_file(f);
    fclose(f);

    if (!record_list)
    {
        fputs(ERROR "Cannot read the record book file.\n", stderr);
        free(file_name);
        return EXIT_FAILURE;
    }

    /* ALLOCATED: file_name, record_list, record */
    record = telephone_book_record_alloc(argv[2], argv[3], argv[4], -1);

    if (!record)
    {
        fputs(ERROR "Cannot allocate memory for the new record.\n", stderr);
        free(file_name);
        telephone_book_record_list_free(record_list);
        return EXIT_FAILURE;
    }

    if (telephone_book_record_list_add_record(record_list, record))
    {
        fputs(ERROR "Cannot add the new entry to the record book.\n", stderr);
        free(file_name);
        telephone_book_record_list_free(record_list);
        telephone_book_record_free(record);
        return EXIT_FAILURE;
    }

    /* Even if the following two functions fail, proceed further. */
    telephone_book_record_list_sort(record_list);
    telephone_book_record_list_fix_ids(record_list);

    f = fopen(file_name, "w");
    free(file_name);

    if (!f)
    {
        fputs(ERROR "Cannot open the record book file.\n", stderr);
        /* 'record' is contained in 'record_list' so is freed by it: */
        telephone_book_record_list_free(record_list);
        telephone_book_record_free(record);
        return EXIT_FAILURE;
    }

    if (telephone_book_record_list_write_to_file(record_list, f))
    {
        fputs(ERROR "Cannot update the record book file.\n", stderr);
    }

    fclose(f);
    /* 'record' is contained in 'record_list' so is freed by it: */
    telephone_book_record_list_free(record_list);
    return EXIT_SUCCESS;
}

/*******************************************************************************
* Handles the commmand for removing records by their IDs.                      *
*******************************************************************************/
static int command_remove_records(int argc, char* argv[])
{
    char* file_name;
    FILE* f;
    telephone_book_record_list* record_list;
    telephone_book_record_list* removed_record_list;
    telephone_book_record* removed_record;
    telephone_book_record_list_node* current_node;
    char* removed_record_format;
    int arg_index;
    int id;

    if (argc < 3)
    {
        puts(WARNING "No record IDs given. Nothing to remove.");
        return EXIT_SUCCESS;
    }

    /* ALLOCATED: file_name */
    file_name = get_telephone_record_book_file_path();

    if (!file_name)
    {
        fputs(ERROR "Cannot allocate memory for the telephone book file name.",
              stderr);
        return EXIT_FAILURE;
    }

    f = fopen(file_name, "r");

    if (!f)
    {
        fprintf(stderr,
                ERROR "Cannot open the record book file '%s'.\n",
                file_name);

        free(file_name);
        return EXIT_FAILURE;
    }

    /* ALLOCATED: file_name, record_list */
    record_list = telephone_book_record_list_read_from_file(f);
    fclose(f);

    if (!record_list)
    {
        fputs(ERROR "Cannot read the record book file.", stderr);
        free(file_name);
        return EXIT_FAILURE;
    }

    /* ALLOCATED: file_name, record_list, removed_record_list */
    removed_record_list = telephone_book_record_list_alloc();

    if (!removed_record_list)
    {
        fputs(ERROR "Cannot allocate memory for the list of removed records.",
              stderr);
        free(file_name);
        telephone_book_record_list_free(record_list);
        return EXIT_FAILURE;
    }

    /* Even if the following two functions fail, proceed further. */
    telephone_book_record_list_sort(record_list);
    telephone_book_record_list_fix_ids(record_list);

    f = fopen(file_name, "w");

    /* ALLOCATED: record_list, removed_record_list */
    free(file_name); /* We do not need 'file_name' anymore. */

    if (!f)
    {
        fputs(ERROR "Cannot open the record book file for writing.", stderr);
        telephone_book_record_list_free(record_list);
        telephone_book_record_list_free(removed_record_list);
        return EXIT_FAILURE;
    }

    for (arg_index = 2; arg_index < argc; ++arg_index)
    {
        if (sscanf(argv[arg_index], "%d", &id) != 1)
        {
            printf(WARNING "Bad ID = \'%s\'. Ignored.\n", argv[arg_index]);
            continue;
        }

        removed_record = telephone_book_record_list_remove_entry(record_list,
                                                                 id);

        if (removed_record)
        {
            telephone_book_record_list_add_record(removed_record_list,
                                                  removed_record);
        }
    }

    if (telephone_book_record_list_write_to_file(record_list, f))
    {
        fputs(ERROR "Cannot update the record book file.", stderr);
        telephone_book_record_list_free(record_list);
        telephone_book_record_list_free(removed_record_list);
        fclose(f);
        return EXIT_FAILURE;
    }

    fclose(f);

    printf(INFO "Number of records to remove: %d, removed: %d.\n",
           argc - 2,
           telephone_book_record_list_size(removed_record_list));

    if (telephone_book_record_list_size(removed_record_list) == 0)
    {
        puts(INFO "Nothing to remove.");
        telephone_book_record_list_free(removed_record_list);
        telephone_book_record_list_free(record_list);
        return EXIT_SUCCESS;
    }

    puts(INFO "List of removed entries:");
    current_node = removed_record_list->head;
    removed_record_format =
        get_removed_record_output_format_string(removed_record_list);

    while (current_node)
    {
        if (removed_record_format)
        {
            printf(removed_record_format,
                   current_node->record->last_name,
                   current_node->record->first_name,
                   current_node->record->telephone_number,
                   current_node->record->id);
        }
        else
        {
            /* Fallback format output: */
            printf("%s, %s - %s, ID %d\n",
                   current_node->record->last_name,
                   current_node->record->first_name,
                   current_node->record->telephone_number,
                   current_node->record->id);
        }

        current_node = current_node->next;
    }

    free(removed_record_format);
    telephone_book_record_list_free(record_list);
    telephone_book_record_list_free(removed_record_list);
    return EXIT_SUCCESS;
}

int main(int argc, char* argv[]) {
    if (argc == 1)
    {
        return command_list_telephone_book_records(argc, argv);
    }

    if (strcmp(argv[1], OPTION_HELP_SHORT) == 0 ||
        strcmp(argv[1], OPTION_HELP_LONG) == 0)
    {
        print_help(argv[0]);
        return EXIT_SUCCESS;
    }

    if (strcmp(argv[1], OPTION_ADD_SHORT) == 0 ||
        strcmp(argv[1], OPTION_ADD_LONG) == 0)
    {
        return command_add_record(argc, argv);
    }

    if (strcmp(argv[1], OPTION_REMOVE_SHORT) == 0 ||
        strcmp(argv[1], OPTION_REMOVE_LONG) == 0)
    {
        return command_remove_records(argc, argv);
    }

    return command_list_telephone_book_records(argc, argv);
}

Critique request

Is there anything else to improve? Please tell me whatever comes to mind.

Improve this question
\$\endgroup\$
0

2 Answers 2

Reset to default
1
\$\begingroup\$

I haven't finished looking at your code, particularly the guts of the actual 'telephone_book.c'. So far, overall the code looks good. It's easy to read and you have clearly put a reasonable amount of work into the code. That said, here are some thoughts I had whilst reading the code.

Return values should indicate one thing

Think carefully out what information you're hiding / returning. I'm running on Windows, and I don't have a HOME environment variable set. When I run the program, rather than getting told something useful to help me diagnose the problem I get told:

Cannot allocate memory for the telephone book file name.

This is because your get_telephone_record_book_file_path is using the return value of NULL to indicate two things. It either indicates that malloc has failed, or it indicates as in my case that the call to getenv failed.

min3

When I first saw your min3 method, my initial impression was that it seemed quite verbose. You've already defined a MAX macro in 'telephone_book_utils.c', why not define a MIN macro:

#define MIN(X,Y) ((X)<(Y)?(X):(Y))

Your min3 then becomes a more concise:

static size_t min3(size_t a, size_t b, size_t c)
{
    return MIN(MIN(a,b), MIN(b,c));
}

compute_edit_distance

This method feels like it could be useful in different scenarios. I wonder if it should perhaps be called compute_Levenshtein_distance and perhaps pushed out of 'main.c' into a utility file so that it can be reused.

Leaking memory

Your command_list_telephone_book_records method is leaking memory. It calls get_telephone_record_book_file_path and assigns the return value to file_name which is cleaned up, however when you call fopen, you're not using the variable you're calling the method again to allocate another copy of the value:

f = fopen(get_telephone_record_book_file_path(), "r");

f what?

On the whole, your variable names are good and descriptive. That said, you're using f consistently for the file handle to the telephone book. Why not give it a proper name?

Duplicate Code

There's quite a lot of duplicate code in 'main.c' to do with reading the phonebook. You have to get the file name, open the file, read from it, close it. This is common to several operations (you need to do it to display the phone book, add to it, remove from it). It feels like it should really be encapsulated in a function so that it can be called rather than being duplicated as the code can get out of sync (showing the phonebook is the only one that contains the memory leak for example).

File Management

I actually think that your API for the phonebook is wrong. Rather than having telephone_book_record_list_read_from_file take in a FILE*, it should take in a filename and the FILE io operations should be hidden behind the scenes. If you were performing record level operations on the file with each call (read entry, rather than read list) then I think working from the FILE* might make sense, however that's not what you're doing. The file operations performed by the read_from_file and write_to_file are tightly coupled to the FILE*. read_from reads from the current position of the file pointer to the end of the file. What happens if the caller opens the file before calling write_to_file in append mode, rather than overwrite mode etc.

Format strings

I'm not a huge fan of TOKEN_SCAN_FORMAT:

read_result = fscanf(f,
                     TOKEN_SCAN_FORMAT
                     TOKEN_SCAN_FORMAT
                     TOKEN_SCAN_FORMAT
                     "%d\n",
                     last_name_token,
                     first_name_token,
                     phone_number_token,
                     &id_holder);

However, if I was going to do it, then for consistency I'd consider adding additional defines for the other two elements of the file (replace %d\n with something like):

ID_SCAN_FORMAT
END_RECORD_SCAN_FORMAT

Header file includes

I know this is subjective, but as a general rule, I don't include headers from other headers (there are some exceptions when advertising a library / using a precompiled header). I then add includes to source files in a particular order, from most general (standard library headers) to most specific (project specific headers). There is a maintenance overhead with this approach (if you introduce a dependency you have to update every source file that includes the header), however I've found that it pays for itself with other benefits, including helping to pinpoint naming conflicts to the most specific file. On large projects it can also make a significant difference to compilation times. You mostly include your local headers first, however this isn't always the case (telephone_book_utils.h). I'd say consistency is key...

Consistent return values

I'd consider defining some constants for your return values (particularly your error conditions). This will encourage consistency across your API and will make the code that little bit easier to read. For example telephone_book_record_list_sort and some other methods return 1 for errors, whereas telephone_book_record_list_size (which doesn't have its error return code documented in the function comment) returns -1. They could probably all be returning -1.

Improve this answer
\$\endgroup\$
0
\$\begingroup\$

I suggest elimination of all the calculation of the 'distance' values

Instead, have a 'configuration' file (perhaps named 'telephone.cfg') that contains three values:

  • max first name length
  • max last name length
  • max phone number length

Then read/update that file as necessary

The configuration file could be initialized to 0,0,0 when the file does not already exist.

The program seems to be missing the ability to handle the case where the phone number has an extension that must be dialed to reach the target person.

Strongly suggest placing some arbitrary max length on each of the fields contents (say 20 characters) so the 'distance' calculations do not ever output a list of entries that is 'effectively' unreadable.

Having a 'max length' may lead to further simplifications of the code to print the phone book. For instance, always using that 'max length' for column width when printing. It would also eliminate the need for a temporary linked list being created when printing.

There does seem to be a problem with the file of telephone entries. For instance, how can the record structure handle those 'pointers' to data. What will be needed in the 'Telephone' file is the actual data, not some pointer to the data (as where that pointer points will no longer exist.)

How is the user expected to ask for a record by 'id' when (per the code) the id's are updated when the data is written from the linked list to the file?

I.E. the id's will change, from one run to the next as records are added/deleted/modified

Improve this answer
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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