Notes on Exam 3: Structures, Sorting, and Such

Contents:

General Notes
Problems
Extra Credit

General Notes

Academic Honesty

It looks like we need to revisit academic honesty as it pertains to code.

We will also discuss what we think appropriate responses are to what appears to be some serious failures to cite.

Documentation

I was sorry to see that many of you neglected various forms of documentation. Remember: Their are three audiences for your code and documentation:

Those who will be using the procedures and libraries you write. We call these your clients.
The person who will be translating your algorithm into code. We call this you.
The person who will be responsible for maintaining or at least understanding your code. (Typically, me. In the real world, this may be you or someone else.) We call this your maintainer.

You clients need documentation that specifies clearly what legal and illegal calls are and what your procedure does in each case.

While writing the code, you should have comments that give the pseudo-code of the algorithm.

Your maintainers need that pseudo-code (what did she plan to do) as well as comments about particular tricks you are using.

This maintainer had to deal with a lot of code that was not obvious at first or second glance and had not documentation to explain it.

Unit Testing

I seem to have neglected a common statement in talking about unit testing: Good testing requires imagination. Failure to test well represents a failure of the imagination.

Even my imagination fails sometimes. For example, although I tried deleting a from lists like (a b c) (a a b c), I did not see what happened with three copies, as in (a a a b b b). At least one of you wrote code that passed the first two, but not the third.

Similarly, in building permutations, I did not envision that someone would say arrays a and b are permutations of each other if all the elements in a appear somewhere in b, all the elements in b appear somewhere in a, and the sum of the elements in a is the same as the sum of the elements in b. If I had realized that, I would have added

  test_ia_is_permutation ((int []) { 1, 2, 2, 2, 3 },
                          (int []) { 1, 1, 2, 3, 3 },
                          5,
                          TRUE);

Problems

Problem 1: Deleting Elements from Lists

Topics: Linked Lists, Structures, Pointers

Code: Examples/Lists

As you may recall, we've recently been working on a new model of lists in which lists are dynamic, indexed, sequences of values. We've started to implement a variety of operations.

For some strange reason, your imaginary classmate, Della 133+ (yes, she's enough of a hacker that she legally had her last name changed to that), is particularly intrigued by the idea of deletion in lists. She decides it would be useful to have a function that deletes all copies of a string from a list.

She even writes a stub with minimal documentation.

/**
 * Delete all elements of str from lst.  Returns whether or not we
 * succeeded.
 */
int
list_delete_all (struct list *lst, char *str)
{
  return 0;     // STUB
} // list_delete_all

Document (six P's), test (unit), and write (implement) list_delete_all.

Some of your tests should be that list_delete_all appropriately frees memory. For example, suppose that mem_allocated returns M. We then add five copies of "whatever" (which does not currently occur in the list) to our list. Adding elements to a list typically allocates some memory, so the next call to mem_allocated should return some N greater than M. Finally, we call list_delete_all to delete all copies of "whatever". At this point, not only should there be no copies of "whatever", mem_allocated should once again return M.

Some Solutions

While it is best to incorporate this code in list.c, I have placed it in separate files for ease of explanation.

Six-P's


#ifndef __LIST_DELETE_ALL__
#define __LIST_DELETE_ALL__

/**
 * list-delete-all.h
 *   The new "list_delete_all" function.  
 */

#include "list.h"

/**
 * Procedure:
 *   list_delete_all
 * Parameters:
 *   lst, a pointer to a list
 *   str, a string
 * Purpose:
 *   Delete all copies of str from lst.
 * Produces:
 *   ok, an integer
 * Preconditions:
 *   lst is not NULL.
 *   str is not NULL.
 *   lst contains N copies of str.  That is, there are N distinct i
 *     s.t. list_get (list, i) is str.
 * Postconditions:
 *   No elements have been added to lst.
 *   No elements have been rearranged in lst.
 *   If the procedure suceeded, ok is true and
 *     lst contains no copies of str.  That is, list_index_of (lst, str)
 *       returns -1.
 *     The length of lst has decreased by N.
 *     No other elements have been rearranged or deleted or added.
 *   If the procedure failed, ok is false and
 *     Some copies of str may have been deleted.
 */
int list_delete_all (struct list *lst, char *str);
 
#endif // __LIST_DELETE_ALL

Unit Tests

We should check deleting in the middle, at the beginning, and at the end. We should make sure that it deals okay when there are multiple elements to delete in a row. We should make sure that there are no memory leaks. Here's some testing that uses some helpers to check whether a list contains all the elements we expect.


/**
 * lda-utest.c
 *   Unit tests for list_delete_all.
 */

// +---------+--------------------------------------------------------
// | Headers |
// +---------+

#include <stdio.h>
#include <string.h>

#include "list.h"
#include "list-delete-all.h"
#include "mem.h"


// +---------+--------------------------------------------------------
// | Globals |
// +---------+

/**
 * The number of tests.
 */
static int tests = 0;

/**
 * The number of errors
 */
static int errors = 0;


// +---------+--------------------------------------------------------
// | Helpers |
// +---------+

static int
string_equal (char *str1, char *str2)
{
  if (str1 == str2)
    return 1;
  else if ((str1 == NULL) || (str2 == NULL))
    return 0;
  else 
    return (strcmp (str1, str2) == 0);
} // string_equal

static void
print_array (char *strings[], int size, FILE *fp)
{
  int i;
  if (size == 0)
    {
      fprintf (fp, "()");
      return;
    }

  fprintf (fp, "(\"%s\"", strings[0]);
  for (i = 1; i < size; i++)
    fprintf (fp, " \"%s\"", strings[i]);
  fprintf (fp, ")");
} // print_array

/**
 * Report info on list and array.  A helper for list_check
 */
static void
report_info (struct list *lst, char *strings[], int size)
{
  fprintf (stderr, "  Expected: ");
  print_array (strings, size, stderr);
  fprintf (stderr, "\n");
  fprintf (stderr, "  Got:      ");
  list_print (lst, stderr);
  fprintf (stderr, "\n");
} // report_info

/**
 * Check if a list has a particular set of strings.  That is, for
 *   all i, 0 <= i < size; list_get (lst, i) is strings[i].
 */
static void
list_check (char *name, struct list *lst, char *strings[], int size)
{
  int i;
  char *str;
  ++tests;
  if (list_length (lst) != size)
    {
      ++errors;
      fprintf (stderr, "Test failed: %s\n", name);
      fprintf (stderr, "  Expected size %d list, got %d.\n", 
               size, list_length (lst));
      report_info (lst, strings, size);
      return;
    }
  for (i = 0; i < size; i++)
    {
      str = list_get (lst, i);
      if (! string_equal (str, strings[i]))
        {
          ++errors;
          fprintf (stderr, "Test failed: %s\n", name);
          fprintf (stderr, "  Got incorrect string at position %d.\n", i);
          report_info (lst, strings, size);
          return;
        } // if
    } // for
} // list_check


// +------+-----------------------------------------------------------
// | Main |
// +------+

int
main ()
{
  int empty_list_allocated;
  struct list *lst;

  lst = list_new ();
  empty_list_allocated = mem_allocated ();
   
  // Deleting from an empty list.
  list_delete_all (lst, "a");
  list_check ("deleting from empty list",
              lst,
              ((char *[]) { }),
              0);

  // Deleting an element not in the list.
  list_append (lst, "a");
  list_append (lst, "a");
  list_append (lst, "b");
  list_append (lst, "b");
  list_append (lst, "c");
  list_append (lst, "a");
  list_append (lst, "d");
  list_append (lst, "c");
  list_append (lst, "c");
  list_delete_all (lst, "e");
  list_check ("deleting element not in list",
              lst,
              ((char *[]) {"a","a","b","b","c","a","d","c","c"}),
              9);

  // Simple deletion, just one element in a list
  list_delete_all (lst, "d");
  list_check ("deleting element from middle of list",
              lst,
              ((char *[]) {"a","a","b","b","c","a","c","c"}),
              8);

  // Delete multiple elements in a row
  list_delete_all (lst, "b");
  list_check ("deleting multiple elements in a row",
              lst,
              ((char *[]) {"a","a","c","a","c","c"}),
              6);

  // Delete elements from the beginning of the list (and elsewher)
  list_delete_all (lst, "a");
  list_check ("deleting elements from front",
              lst,
              ((char *[]) {"c","c","c"}),
              3);

 // Delete elements from the end of the list
 list_prepend (lst, "b");
 list_delete_all (lst, "c");
 list_check ("deleting elements from end",
             lst,
             ((char *[]) {"b"}),
             1);

  // Delete all elements
  list_delete_all (lst, "b");
  list_check ("deleting all remaining elements",
              lst,
              ((char *[]) { }),
              0);

  // Check memory allocation
  ++tests;
  if (mem_allocated () != empty_list_allocated)
    {
      ++errors;
      fprintf (stderr, "Error: Memory leak.\n");
    }

  // Make sure that we can still append values.
  list_append (lst, "z");
  list_prepend (lst, "a");
  list_append (lst, "y");
  list_check ("appending elements",
              lst,
              ((char *[]) {"a","z","y"}),
              3);

  // And we're done
  fprintf (stderr, "%d tests conducted.\n", tests);
  fprintf (stderr, "%d errors encountered.\n", errors);

  return errors;
} // main

Implementation 1: Use Helpers

To delete all copies of an element from a list we have to find each copy and then delete the copy. The first approach one often uses is to think about what resources are available. We already have two useful procedures for these two ideas:

list_index_of (lst, str) finds out where in lst str appears.
list_cut (lst, index) cuts the element at a particular index.

So, we implement list_delete_all by repeatedly finding the index and cutting it out.


int
list_delete_all (struct list *lst, char *str)
{
  int pos;
  while ((pos = list_index_of (lst, str)) != -1)
    {
      if (! list_cut (lst, pos))
        return 0;
    } // while
  return 1;
} // list_delete_all

This solution is a wonderful use of programmer time. However, it is not very efficient, since we have to restart at the beginning of the list each time we delete an element. (E.g., if it turns out that the element to delete appears at positions 100, 101, and 102 of the original list, we revisit the elements at positions 0..99 four times.) Hence, we might look for other solutions.

Implementation 2: Scheme Style

To make this more efficient, we might think about how we can delete each element in turn. To do so, we might step backwards and think about how we implemented delete-all in Scheme.

(define delete-all
  (lambda (lst str)
    (cond
      ((null? lst) null)
      ((equal? (car lst) str)
       (delete-all (cdr lst) str))
      (else 
       (cons (car lst) 
             (delete-all (cdr lst) str))))))

Now, we don't really want to use cons, since that builds a new node, thereby creating a memory leak. Hence, we would need to change lst, rather than cons'ing again.

(define delete-all
  (lambda (lst str)
    (cond
      ((null? lst) null)
      ((equal? (car lst) str)
       (delete-all (cdr lst) str))
      (else 
       (set-cdr! lst (delete-all (cdr lst) str))))))

So, to delete from a wrapped list, we call a translation of this code on lst->head.

One problem is that we may be deleting from the tail. Hence, we also need to update the tail. That basically requires going through the list again. We also need to find the length.

Putting it all together, we get


/**
 * Delete all copies of str from the node list np.
 */
struct node *
np_delete_all (struct node *np, char *str)
{
  // You can't delete from the empty list
  if (np == NULL)
    return NULL;

  // Remove all copies of str from the rest of the list
  struct node *newcdr = np_delete_all (cdr (np), str);

  // Is str at the front of the list?  If so, delete the first element
  // and return the modified cdr.
  if (strcmp (car (np), str) == 0)
    {
      free (np);
      return newcdr;
    }
  // Otherwise, just update the cdr
  else
    {
      set_cdr (np, newcdr);
      return np;
    }
} // np_delete_all

/**
 * Find the last node in a non-null list.
 */
struct node *
np_last_node (struct node *np)
{
  while (np->next != NULL)
   np = np->next;
  return np;
} // np_last_node

int
list_delete_all (struct list *lst, char *str)
{
  // Delete the elements
  lst->head = np_delete_all (lst->head, str);

  // Update the tail
  if (lst->head == NULL)
    lst->tail = NULL;
  else
    lst->tail = np_last_node (lst->head);

  // Update the length
  lst->length = np_length (lst->head);

  // And we're done
  return 1;
} // list_delete_all

That's a bit better. But it still requires us to iterate the list three times: Once to delete elements, once to find the potentially new tail, and once to update the length. Perhaps we should do that all at once.

Implementation 3: Direct Implementation

We handle the normal case by iterating through the list, checking the value of the current element.

  current = lst->head;
  while (current != NULL)
    {
      ...
      current = cdr (current);
    }

Of course, it is hard to delete a node when we're at the node. Hence, we also need to keep track of the previous node.

  current = lst->head;
  while (current != NULL)
    {
      ...
      previous = current;
      current = cdr (current);
    }

As we've done in many other procedures, we'll also need to consider a variety of special cases. Special cases include:

We might delete from the head of the list.
We might delete from the tail of the list.
We might delete from the empty list.

We handle the first and third special case by looking at the head and, as long as it is not-null and contains string, advance it and delete the old head.

We handle the second special case by using the previous value. When current is NULL, previous must be the end of the list.

  lst->tail = previous;

Putting it all together, we get


int
list_delete_all (struct list *lst, char *str)
{
  struct node *garbage;	        // Something we want to delte
  struct node *current;         // For iterating lists
  struct node *previous;        // Also for iterating lists
 
  // Special case: The value is at the head
  while ((lst->head != NULL) && (strcmp (car (lst->head), str) == 0))
    {
      garbage = lst->head;
      lst->head = cdr (lst->head);
      free (garbage);
      --(lst->length);
    } // while

  // At this point, we know that the head of the list does not contain
  // str.  However, it may be empty.
  if (lst->head == NULL)
    {
      lst->tail = NULL;
      lst->length = 0;
      return 1;
    }

  // If we reach this point, the list is nonempty, and the head does not 
  // contain str.
  previous = lst->head;
  current = lst->head->next;

  // Keep going until we reach the end of the list
  while (current != NULL)
    {
      // If the current element contains str
      if (strcmp (car (current), str) == 0)
        {
          // Delete it
          garbage = current;
          current = cdr (current);
          free (garbage);
          set_cdr (previous, current);
          --(lst->length);
        }
      // If the current element does not contain str, go on
      else
        {
          previous = current;
          current = cdr (current);
        }
    } // while

  // Update the tail
  lst->tail = previous;

  // And we're done.
  return 1;
} // list_delete_all

Notes

The Six P's

Writing good documentation for this is hard. In particular, it's hard to say formally "other elements remain in order". I still looked for you to say something like that informally. Here's a postcondition that does not say that.

  lst is modified so that it does not have any copies of str.  Any
  memory associated with storing copies of str has been freed.

But I can meet those postconditions with

int 
list_delete_all (struct list *lst, char *str)
{
  list_clear (lst);
  return 1;
} // list_delete_all

Some of you wrote that if problems occured, the list is unmodified. Unfortunately, problems are likely to occur midway through deleting, which means that the list will be modified.

Unit Tests

Many of you had somewhat limited unit tests. Important tests that were often skipped. Here are ones that I hoped to see.

Can we still append and prepend after deleting? (Many of you cleared the list between tests, which serves as a form of reset.)
What happens if multiple copies of the element to be deleted appear at the front of the list?
What happens if multiple copies of the element to be deleted appear in sequence in the middle of the list?
What happens if multiple copies of the element to be deleted appear in sequence at the end of the list?
Does it leak memory?

Implementation

A few of you wrote something like the following:

  struct node *current = lst->head;

  // Check the head!
  if (strcmp (car (lst->head), str) == 0)
    {
      struct node *garbage = list->head;
      list->head = cdr (list->head);
      free (garbage);
    }
  ...
  current = current->next;

Note that the use of current->next is dangerous. Since current points to the old head, and we've freed that, we should not make any assumptions about what is still left at that place in memory.

Problem 2: Checking Permutations

Topics: Arrays, Sorting, Unit Testing

Code: Examples/Exam3/Perm

As you may recall, your imaginary classmates, Terrence and Theresa Tester, like writing unit tests. In order to test sorting routines, they know that they need two operations, one to check whether an array is sorted, and one to check whether an array is a permutation of another. (After all, sorting routines are supposed to produce arrays that are in order and are permutations of the original array.)

They produce a declaration of one version of the is it a permutation function.


#ifndef __PERM_H__
#define __PERM_H__

/**
 * perm.h
 *   Declarations for some utilities for fun with permutations.
 */

// +--------------------+---------------------------------------------
// | Exported Functions |
// +--------------------+

/**
 * Procedure:
 *   ia_is_permutation
 * Parameters:
 *   a0, an array of integers
 *   a1, an array of integers
 *   size, an integer
 * Purpose:
 *   Determine if the subarray of the first size elements of a1 is a 
 *   permutation of the subarray of the first size elements of a0.
 * Produces:
 *   is_perm, an integer representing a Boolean value
 * Preconditions:
 *   a0 contains at least size elements.
 *   a1 contains at least size elements.
 * Postconditions:
 *   a0 is unmodified
 *   a1 is unmodified
 */
int ia_is_permutation (int a0[], int a1[], int size);

#endif // __PERM_H__

Of course, as people who love testing, they write tests of ia_is_permutation, which you can find in perm-utest.c.

However, they seem to be unable to write the definition of ia_is_permutation.

Write that definition. That is, implement ia_is_permutation. You may find it useful to add tests.

While you may be tempted to decide if one array is a permutation of another by sorting both arrays and comparing the results, that strategy is inappropriate if we plan to use the function to test sorting algorithms.

A Solution

The obvious strategy is to step through the first array, checking to see whether the second array contains each element.

  // Look at each element in a0
  for (i = 0; i < size; i++)
    {
      // Make sure the corresponding element is in a1
      if (! ia_contains (a1, a0[i]))
        return 0;
    } // for
  // Everything's there.  We're done
  return 1;

But that strategy fails for the arrays {1,1,2} and {1,2,2}. That is, all the values in the firat array appear in the second array, but not an equal number of times. So we have to do a bit more.

Tallying

One solution is to count the number of times each element appears in each array. If each element in a0 appears the same number of times in both a0 and a1, then the two arrays are permutations of each other.


/**
 * perm.c
 *   Definitions of functions useful for checking permutations.
 */

// +---------+--------------------------------------------------------
// | Headers |
// +---------+

#include "perm.h"
#include "bool.h"


// +---------+--------------------------------------------------------
// | Helpers |
// +---------+

/**
 * Count how many times val appears in the first size elements of vals. 
 */
static int
ia_tally (int vals[], int size, int val)
{
  int i;
  int tally = 0;
  for (i = 0; i < size; i++)
    if (vals[i] == val)
      ++tally;
  return tally;
} // ia_tally


// +--------------------+---------------------------------------------
// | Exported Functions |
// +--------------------+

int
ia_is_permutation (int a0[], int a1[], int size)
{
  int i;
  // For each element in a0
  for (i = 0; i < size; i++)
    {
      // Make sure that the element appears the same number of times in 
      // a0 and a1.
      if (ia_tally (a0, size, a0[i]) != ia_tally (a1, size, a0[i]))
        return 0;
    } // for
  // If all tallies are equal, the two arrays are permutations
  return 1;
} // ia_is_permutation

Checking Off Values

If you think about how we would do this exercise by hand, we probably wouldn't count how many times each value appears. Rather, we would check off each value in a0 from a1. We'll use an extra array, checkmarks, to see whether or not we've checked off a value.


/**
 * perm.c
 *   Definitions of functions useful for checking permutations.
 */

// +---------+--------------------------------------------------------
// | Headers |
// +---------+

#include "perm.h"
#include "bool.h"


// +---------+--------------------------------------------------------
// | Helpers |
// +---------+

/**
 * Check off one copy of val from vals, assuming that the elements of
 * checkmarks indicate whether or not we've checked off the corresponding
 * value in vals.
 */
static int
check_off (int vals[], int checkmarks[], int size, int val)
{
  int i;
  for (i = 0; i < size; i++)
    {
      if ((! checkmarks[i]) && (vals[i] == val))
        {
          checkmarks[i] = 1;
          return 1;
        }
    } // for
  // If we've gotten this far, we didn't check anything off
  return 0;
} // check_off


// +--------------------+---------------------------------------------
// | Exported Functions |
// +--------------------+

int
ia_is_permutation (int a0[], int a1[], int size)
{
  int checkmarks[size];
  int i;

  // Initially, nothing is marked
  for (i = 0; i < size; i++)
    checkmarks[i] = 0;

  // For each element of a0
  for (i = 0; i < size; i++)
    {
      // Try to check it off in a1
      if (! check_off (a1, checkmarks, size, a0[i]))
        return 0;
    } // for

  // Yay!  We checked off every element.  We're done
  return 1;
} // ia_is_permutation

Notes

Problem 3: Erroneous String Code

Topics: Strings, Lists, Memory, Segfaults, Debugging, File Input

Code: Examples/Exam3/Strings

Your imaginary colleagues, Strongheart and Struana, have had some difficulties with their string programming. They've written the following program, which seems to segfault a lot. Even when it doesn't segfault, it seems to give them strange output. For example, although they think they've read all the lines from the Makefile, they don't seem to see all of those lines.


/**
 * uc.c - Fun with uppercase.  Takes a variety of strings (from the command
 *   line, from the Makefile or stdin, elsewhere), converts them to uppercase,
 *   and then prints them out.
 *
 * Sample command:
 *   ./uc hello goodbye
 */

// +---------+--------------------------------------------------------
// | Headers |
// +---------+

#include <ctype.h>	// For islower
#include <stdio.h>	// For fun with input and output
#include <string.h>	// For strchr

#include "list.h"	// We're working with lists


// +-----------+------------------------------------------------------
// | Constants |
// +-----------+

// The maximum length of a string
#define MAXLEN 128


// +---------+--------------------------------------------------------
// | Helpers |
// +---------+

/**
 * Convert a string to uppercase.
 */
int
uc (char *str)
{
  static int offset = 'A' - 'a';

  // For each character in the string
  while (*str != '\0')
    {
      // If the character is lowercase
      if (islower (*str))
        // Make it uppercase
        *str = *str + offset;
      // And move on to the next character.
      str++;
    } // while

  // Return 1 to match the requirements for foreach
  return 1;
} // uc


/**
 * Print out a horizontal rule of a particular length.
 */
void
hrule (int len)
{  
  int i;
  for (i = 0; i < len; i++)
    putchar ('-');
  putchar ('\n');
} // hrule

/**
 * Print out a string on a line by itself.
 */
int
print_string (char *str)
{
  return printf ("%s\n", str);
} // print_string


// +------+-----------------------------------------------------------
// | Main |
// +------+

int
main (int argc, char *argv[])
{
  struct list *lst;             // A list of strings.  
  char *line;                   // One line (for input)
  int i;                        // A favorite counter variable

  // Build the list.
  lst = list_new ();
  if (lst == NULL)
    {
      fprintf (stderr, "Could not create a list.  Giving up!\n");
      return 1;
    }

  // Add all of argv.
  for (i = 1; i <= argc; i++)
    list_append (lst, argv[i]);

  // Add some simple strings, such as our names.  Include some lc and uc
  // and non-chars.
  list_append (lst, "Strongheart");
  list_append (lst, "Strauna");
  list_append (lst, "Green Eggs and Green Ham");
  list_append (lst, "Latin@ is a *very* strange word.  I'd give it a 8.");

  // Add the empty string
  list_append (lst, NULL);

  // Add the lines of the Makefile.  (The Makefile is a file we know should
  //   be in the current directory.)
  FILE *fp = fopen ("Makefile", "r");
  if (fp == NULL)
    {
      fprintf (stderr, "Could not open the Makefile.  Giving up.\n");
      return 2;
    }
  while (fgets (line, MAXLEN, fp) != NULL)
    {
      // Drop the \n.
      *(strchr(line, '\n')) = '\0';
      // Add it to the list.
      list_append (lst, line);
    }

  // Print out all of the strings
  hrule (80);
  list_foreach (lst, print_string);

  // Convert all of the strings to uppercase.
  list_foreach (lst, uc);

  // Print out all of the strings again
  hrule (80);
  list_foreach (lst, print_string);

  // And we're done
  return 0;
} // main

Repair their code. That is, you should not only stop the code from crashing, you should ensure that it meets the goals specified in the comments.

A Solution

Let's address their problems one by one.

Command Line Processing

  for (i = 1; i <= argc; i++)
    list_append (lst, argv[i]);

An off-by-one error. This should be

  for (i = 1; i < argc; i++)
    list_append (lst, argv[i]);

Simple Strings

  list_append (lst, "Strongheart");
  list_append (lst, "Strauna");
  list_append (lst, "Green Eggs and Green Ham");
  list_append (lst, "Latin@ is a *very* strange word.  I'd give it a 8.");

We plan to mutate the strings in lst by calling uc on them. Hence, we cannot use constant strings. There are a few primary options.

a. We can put the constant strings in arrays, making them mutable.

  char strongheart[] = "Strongheart";
  list_append (lst, strongheart);

b. We can allocate mutable memory and copy into the mutable memory.

  char *str = malloc (strlen ("Strongheart") + 1);
  strcpy (str, "Strongheart");
  list_append (lst, str);

c. We can use the handy-dandy strdup function that allocates mutable memory and copies into it.

  list_append (lst, strdup ("Strongheart"));

The Empty String

  // Add the empty string
  list_append (lst, NULL);

NULL is not the empty string. "" is the empty string. Since we may be mutating it, we may even want to make a copy.

   list_append (lst, strdup (""));

Reading Lines

  char *line;                   // One line (for input)
  ...
  while (fgets (line, MAXLEN, fp) != NULL)
    ...

Reading into an un-allocated line is a bad idea. We can allocate line as an array or using malloc. I prefer the former.

  char line[MAXLEN];

Duplicated Lines

  while (fgets (line, MAXLEN, fp) != NULL)
    {
      ...
      // Add it to the list.
      list_append (lst, line);
    }

Since we keep adding line to the list, whenever we change line, we change all the copies of line in the list.

Hence, when we want to read a new line, we need more memory. We can allocate the memory retrospectively, using strdup, or prospectively, using malloc.

a. Retrospectively

      list_append (lst, strdup (line));

b. Prospectively

  char *line;
  ...
  line = malloc (MAXLEN);
  while (fgets (line, MAXLEN, fp) != NULL)
    {
      ...
      // Add it to the list.
      list_append (lst, line);
      // And allocate the next copy
      line = malloc (MAXLEN);
    } 
  // That last line was unused, so drop it
  free (line);

Lines without newline

      // Drop the \n.
      *(strchr(line, '\n')) = '\0';

In a few cases, we won't read a newline. In this case, strchr returns NULL and the dereference is unsafe. We should check the value first.

      // Drop the \n.
      char *newline = strchr (line, '\n');
      if (newline)
        *newline = '\0';

This last one was subtle enough that I did not take off points for you missing it.

Notes

I had intended this problem to be a straightforward exploration of errors you had encountered or that K&R had explicitly or implicitly warned you about. It appears that many of you did not find it that way. I hope that you are now more familiar with these common errors.

Some of you were a bit excessive in your allocation. For example,

  char line[MAXLEN][256];

It's really only necessary to allocate each line when you need it.

However, if you're going to use a strategy like this do not hardcode constants. Use macros when you want a special value.

Problem 4: Sorting Complex Structs

Topics:: Structures, Sorting, Function Pointers

Code: Examples/Exam3/Students

Your imaginary classmates, Stu and Denton, have defined a simple struct to represent students.

struct student 
  {
    char *fname;        // First name
    char *lname;        // Last name
    char *sid;          // Student id number
    double gpa;         // Grade point average
    int debt;           // Debt to the College, in cents
  };

They even create a method that creates new pointers to student structs.

struct student *
student_new (char *fname, char *lname, char *sid, double gpa, int debt)
{
  struct student *result = (struct student *) malloc (sizeof (struct student));
  if (result == NULL)
    return NULL;
  result->fname = fname;
  result->lname = lname;
  result->sid = sid;
  result->gpa = gpa;
  result->debt = debt; 
  return result;
} // student_new

And they use that method to set up a template for a simple program that creates a bunch of students, processes them, and prints them out. (Okay, no processing currently happens.)


/**
 * student-experiments
 *   Some experiments with students.
 */

// +---------+--------------------------------------------------------
// | Headers |
// +---------+

#include "student.h"	// Duh.
#include <stdio.h>	// For printf, stdout, and such


// +------+-----------------------------------------------------------
// | Main |
// +------+

int 
main ()
{
  // Set up a group of sample students.
  struct student *class[] =
    {
      student_new ("John", "Doe", "00001234", 3.4, 0),
      student_new ("John", "Ode", "10203040", 2.0, 100000),
      student_new ("Jane", "Doe", "00001233", 3.9, 1005000),
      student_new ("Jack", "Doe", "00112233", 3.2, 234012),
      student_new ("Jan",  "Edo", "20199999", 2.7, 123450),
      student_new ("Sam",  "Are", "20199998", 1.5, 50),
      student_new ("Sam",  "IAm", "20199908", 3.5, 543210),
      student_new ("Jane", "IAm", "20199908", 3.5, 543210)
    };
  int num_students = sizeof (class) / sizeof (struct student *);

  // Do something with them.  (Say, sort them.)

  // Print them out.
  int i;
  for (i = 0; i < num_students; i++)
    {
      printf ("%2d: ", i);
      student_print (class[i], stdout);
      printf ("\n");
    } // for each student

  // And we're done.
  return 0;
} // main

Now it's time to do something interesting. Extend their program so that it is possible to sort the array of students by name (normal mechanism: prioritize last name, but go to first name if the last names are the same), student-id number, gpa, or debt. It is probably easiest if you do all four kinds of sorting. That is: Print the class, sort the class by name, print it again, sort the class by student-id, print it again, sort the class by gpa, print it again, sort the class by debt, and print it again.

I would recommend that you (a) write a sorting routine that takes a comparator as a parameter and (b) build comparators for those four sorting policies.

Here's a sample signature for your sorting routine.

void
sort_students (struct student *students[], int size, 
               int (* compare) (struct student *, struct student *));

And here's a sample comparator.

int
compare_students_by_gpa (struct student *s, struct student *t)
{
  if (s->gpa < t->gpa)
    return -1;
  else if (s->gpa > t->gpa)
    return 1;
  else
    return 0;
} // compare_students_by_gpa

A Solution

K&R have given us a nice, generic, Quicksort routine, so let's use it. We'll change it in two ways. First, we'll provide a wrapper that meets the signature I asked for. Second, we'll make it work with arrays of student structs, rather than an array of generic pointers.


/**
 * student-sort.c -
 *   K&R's Quicksort routine, updated for our arrays of students
 *   Additional documentation by SamR.
 */

// +---------+--------------------------------------------------------
// | Headers |
// +---------+

#include "student-sort.h"	// Make sure we match the signature


// +---------+--------------------------------------------------------
// | Helpers |
// +---------+

/**
 * swap (students, i, j): Swap the elements in positions i and j.
 */
static void
swap (struct student *students[], int i, int j)
{
  struct student *tmp = students[i];
  students[i] = students[j];
  students[j] = tmp;
} // swap

/**
 * kr_qsort: sort students[left]...students[right] into increasing order using
 *   comp to compare elements
 */ 
void 
kr_qsort (struct student *students[], 
          int left, int right, 
          int (*comp)(struct student *s1, struct student *s2))
{
  int i, last;

  // If the array contains fewer than two elements, do nothing.
  if (left >= right) 
    return;

  // Use the middle element as the pivot, which we put at the left.
  swap(students, left, (left + right)/2);
  last = left; 

  // Rearrange the elements so that large things are on the right
  for (i = left+1; i <= right; i++) 
    if ((*comp)(students[i], students[left]) < 0)
      swap(students, ++last, i); 

   // Put the pivot in the middle
   swap(students, left, last);

   // And recurse
   kr_qsort (students, left, last-1, comp); 
   kr_qsort (students, last+1, right, comp);
} // kr_qsort



// +--------------------+---------------------------------------------
// | Exported Functions |
// +--------------------+

void 
sort_students (struct student *students[], int size,
               int (*compare)(struct student *s, struct student *t))
{
  kr_qsort (students, 0, size-1, compare);
} // sort_students

Then we need to write the comparators. Most of them are straightforward. The only one that needs some thought is by name, and for that we just check the last name first.


/**
 * student-compare.c
 *   Some simple comparators for students.
 */

// +---------+--------------------------------------------------------
// | Headers |
// +---------+

#include <string.h>

#include "student.h"
#include "student-compare.h"


// +-------------+----------------------------------------------------
// | Comparators |
// +-------------+

int 
compare_students_by_debt (struct student *s, struct student *t)
{
  if (s->debt < t->debt)
    return -1;
  else if (s->debt > t->debt)
    return 1;
  else
    return 0;
} // compare_students_by_debt

int
compare_students_by_gpa (struct student *s, struct student *t)
{
  if (s->gpa < t->gpa)
    return -1;
  else if (s->gpa > t->gpa)
    return 1;
  else
    return 0;
} // compare_students_by_gpa

int 
compare_students_by_name (struct student *s, struct student *t)
{
  int lcmp = strcmp (s->lname, t->lname);
  // Normal case: Different last names
  if (lcmp)
    return lcmp;
  // Special case: Identical last names
  else
    return strcmp (s->fname, t->fname);
} // compare_students_by_name

int 
compare_students_by_student_id (struct student *s, struct student *t)
{
  return strcmp (s->sid, t->sid);
} // compare_students_by_student_id

Of course, if you really want to think like a C programmer, you can write compare_students_by_name as

int
compare_students_by_name (struct student *s, struct student *t)
{
  return strcmp (s->lname, t->lname) || strcmp (s->fname, t->fname);
} // compare_students_by_name

Notes

A number of you used Bubble Sort for your sorting routine. There's a reason that we don't teach Bubble Sort - in practice, it is the slowest of the common sorting routines. Do not use Bubble Sort!

Here is how a few of you wrote swap. The code is taken from a student who should remain anonymous. That student tells me that it got the code from http://www.cs.cornell.edu/Info/Courses/Fall-97/CS113/ass/sol2.txt.

//swapping s1 and s2
void swap(struct student *s1, struct student *s2)
{
  struct student *temp = (struct student *) malloc (sizeof (struct student));

  temp->fname = s1->fname;
  temp->lname = s1->lname;
  temp->sid = s1->sid;
  temp->gpa = s1->gpa;
  temp->debt = s1->debt;

  s1->fname = s2->fname;
  s1->lname = s2->lname;
  s1->sid = s2->sid;
  s1->gpa = s2->gpa;
  s1->debt = s2->debt;

  s2->fname = temp->fname;
  s2->lname = temp->lname;
  s2->sid = temp->sid;
  s2->gpa = temp->gpa;
  s2->debt = temp->debt;

  free(temp);
}

This strategy is bad! Why? Consider the following:

  struct student *johnD = 
    student_new ("John", "Doe", "00001234", 3.4, 0);
  struct student *johnO =
    student_new ("John", "Ode", "10203040", 2.0, 100000);
  struct student *janeD =
    student_new ("Jane", "Doe", "00001233", 3.9, 1005000);
  struct student *jackD =
    student_new ("Jack", "Doe", "00112233", 3.2, 234012);
  struct student *janE =
    student_new ("Jan",  "Edo", "20199999", 2.7, 123450);
  struct student *samR =
    student_new ("Sam",  "Are", "20199998", 1.5, 50);
  struct student *sammy =
    student_new ("Sam",  "IAm", "20199908", 3.5, 543210);
  struct student *class =  {johnD, johnO, janeD, jackD, janE, samR, sammy } ;
  ...
  sort_students (class, num_students, compare_students_by_name);
  ...
  printf ("Here's the info for SamR: ");
  print_student (samr, stdout);
  printf ("\n");

Extra Credit

If you made it this far in the notes and are one of the first few people to tell me that you have reached this point in the notes, you may receive a bit of extra credit.

History

Monday, 8 May 2011 [Samuel A. Rebelsky]

Released.
This verion available at http://www.cs.grinnell.edu/~rebelsky/Courses/CSC161/2011S/Exams/notes.03.html