A telephone book command line program in ANSI C

(See the next iteration.)

Introduction

I was in the mood for some C code and wrote this program for handling a personal telephone record book. One of the goals is strict portability; the code compiles with gcc -ansi -pedantic -Wall ... silently. The second goal is being prepared for dealing with problems (out of memory, file I/O errors).

Usage

The program creates and works on a file called .telephone_book that is placed in the user's home directory.

In order to add new record:

./tbook --add Claus Santa 040-1234567


In order to list all records:

./tbook # list all
./tbook blah # list all records whose last name is closest to "blah" by Levenshtein distance
./tbook - blah # list all records whose first name is closest to "blah" by Levenshtein distance
./tbook bluh blah # list all records whose both last and first names are closest to "bluh blah" by Levenshtein distance


The output may be something like:

----------+------------+------------------+---
Last name | First name | Telephone number | ID
----------+------------+------------------+---
Bro | Vector | 11 | 1
Efremov | Rodion | 8055 | 2
Funk | Funky | 12321 | 3
----------+------------+------------------+---
Gates | Bill | 677 | 4
Minogue | Kylie | 4401 | 5
Ryazanov | Viktor | 3454 | 6


In order to delete records by their IDs:

./tbook --remove ID1 ID2 ... IDn


Here are all 5 source files:

telephone_book.h:

`#ifndef TELEPHONE_BOOK_H
#define TELEPHONE_BOOK_H

#include

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

/****

You seem to have built in a cultural assumption that everybody has a first name and a last name, which is not true, and that first and last have the same role everywhere. The changes to fix that are probably quite extensive, unfortunately.

You're not consistent with your include ordering. I normally recommend that you include your own headers first, and Standard Library headers last - this helps identify where your own headers may have made unjustified assumptions that standard headers were included first.

On a related note, you might want to move telephone_book_record_list_write_to_file() and telephone_book_record_list_read_from_file() into a different header (perhaps called telephone_io.h), to avoid telephone.h dragging in ` to every translation unit that includes it. Unfortunately, you can't just provide a compatible definition for FILE, or that would be preferable.

char separator();

Is there a reason you made this a function, rather than a simple constant?

In loops like this:

for (i = 0; i != max_telephone_contact_id_length + 1; ++i)

I normally prefer to use ):

char *write_separator(char *s, char c, int n) {
    memset(s, c, n);
    return s+n;
}

I generally avoid this sort of thing at the low levels:

if (!separator_string)
{
    fputs("ERROR: Cannot allocate memory for the row separator string.",
          stderr);
    return NULL;
}

Prefer to return an error indication, and leave it up to the calling code to decide how to report it. In a GUI version, you probably want to display a GUI message rather than reporting to stderr.

Also, note that you don't free the successfully allocated memory in any of these error paths, giving a memory leak that's hard to track down.

This looks contorted:

sprintf(record_format_string,
        "%%-%zus | %%-%zus | %%-%zus | %%-%zuzu\n",

It's probably clearer to use %*s in the format string, and pass the length with each call.

This one's really weird:

title_string =
        malloc(sizeof(char) *
               (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);

sizeof (char) is 1 by definition; multiplying by it makes you look ignorant. Also, it's not clear why only the last-name token should be multiplied by 1 - the line breaks and parentheses serve to deceive the reader here! It may be simpler and more consistent to ask snprintf` to compute that sum for you using the same format string you'll be printing with (at a small performance cost).

StackExchange Code Review Q#142716, answer score: 5