Program 8A: Sort a Linked List

Write a program to sort a linked list

Given Requirements

 

Requirements: Write program 8a to sort a linked list of n real numbers in ascending order. Where the list items have multiple fields, provide the capability to sort on any field. Assume that the list is short enough to fit in computer memory.

Testing: Thoroughly test the program. As one test, sort the data in the right two columns of table D14. Do two sorts, one on each field, and submit a test report describing both results. Note: your program need not print the results.

Table 9-1. D14: Pascal Object LOC

Object numberObject LOCLoc/Method
1268.67
2397.80
3268.67
4537.57
5175.67
6268.67
7227.33
8538.83
93110.33
10186.00
11258.33
12307.50
134214.00
14459.00
154010.00
1619539.00
1711438.00
188721.75
198729.00
20258.33
215313.25
224711.75
23585.80
24497.00
258814.67
268216.40
27465.75
285513.75
293210.67
30186.00
318929.67
32186.00
333612.00
3416140.25
3511237.33
368922.25
378528.33
388220.50
398216.40
40467.67
4118436.80
4214524.17
439619.20
4412341.00
4521130.14
4640550.63
475814.50
4826452.80
495819.33
5030525.42
 
--[Humphrey95] 

Planning

Requirements

Ouch!

Here's an example of an early design decision which will cost me a bit. Back in the early PSP exercises, I had a number_list class which handled basic number list activities-- sum, mean, standard deviation, and the like. When the time came to expand that class to handle pairs of numbers, I had a choice to make: either keep the existing class and use it twice (a list of x-numbers and a list of y-numbers), or expand the existing class to handle entries of more than one field.

I chose the first option, because it made for an easier implementation, the paired_number_list used in several of the PSP programs. However, I'm about to pay the price now, since program 8a requires the ability to sort on any field.

I could simply rewrite the paired_number_list class to sort on either the xs or the ys members, but there are problems with that approach. First, it's not clean-- I'll have to write separate routines for sorting on either field. Second, it's not extendible-- if we add another field, or more (which we'll do in exercise 10), the amount of work involved begins to increase dramatically.

As a result, I'm going to abandon our long-favored paired_number_list and number_list classes; program 8a should be able to take data files of any number of fields per entry, and sort them appropriately. This requires a slightly different format for the data file; it will still strip comments and whitespace, but will take first a number indicating the number of fields per record, then the records as comma-separated values, then the word "stop". Some code can be reused from the predictor_parser class to do this. The data file format will look something like this:

--test data for program 8a: Sort a linked list (of pairs of numbers)
2 -- number of fields in the record
26, 8.67
39, 7.80
   ...
58, 19.33
305, 25.42
stop -- end of data

Error handling and input format will be the same as previous programs. For testing, program 8a will output the data once per number of fields, sorted by the appropriate field (ie first sorted by field 0, then by field 1, etc.), as well as a flag indicating whether or not the program believes it is sorted.

Size Estimate

A rough conceptual design shows one new class, number_array_list, a calculation class, with approximately 18 new methods, and one new class, number_array_list_parser, which has effectively two new methods (the rest being heavily reused from the old predictor_parser class). This gives us a PROBE-generated estimate of 345 new or changed LOC.

Resource/Time Estimate

Using historical data, I get a PROBE-generated estimate of 174 minutes for program 8a.

Development

Design

Again, using Dia, I've made a quick "class diagram" with numerous notes indicating implementation choices. I found that merely having and using such a tool is forcing me to make more implementation decisions early, which is not necessarily a bad thing-- but it's not necessarily a good thing either. I disapprove of any tool which allows a disparity between the implementation of a design (ie source code files) and the representation of a design (ie the design document) without some mechanism for automatically synchronizing them (like the "round-trip engineering" vaunted by ISE in their EiffelCase tool, an excellent idea whether or not an implementation exists).

Preliminary design diagram, using a form of UML

The design; here, I use recursion a great deal rather than iteration, which makes for a bit more simple design... but a severe decrease in run-time efficiency. Since I'm not expected to sort very large lists, I'll accept the decline in efficiency in favor of a simple, easy-to-maintain design.

Design Review

I spent long enough on the design that the design review was painful; perhaps I should have taken more of a break. At any rate, it did find some significant features missing, wrong algorithms, etc. It's difficult, as Humphrey predicted, to get motivated about the design and code reviews-- they seem to take so much time without producing much product. Still, they seem to be worthwhile-- and I'm still unskilled enough at them that I'm missing a great number of errors.

Code

Since I spent so much time on detailed design, coding was particularly straightforward-- but not any shorter! Just taking that result, it seems that a detailed design leads to longer development time, given the tools I have-- because even if you refined the design down to actual code, you'd still have to type it in (thus my interest in something which can translate the design into code).

Dia is fairly scriptable, and saves its results in an easy-to-parse XML format. It would probably be relatively easy to write a parser which could generate at least a code skeleton from a Dia diagram. But this has problems-- it mixes design and implementation a bit, and cannot reliably reverse-engineer a set of C++ classes into UML.

I have more hope for BON/Eiffel, which are closely related (as UML and C++ are, imho, despite UML's claim of language independence). It might be relatively easy to add BON shapes to Dia and write a parser which could indeed forward- and reverse-engineer Eiffel code from Dia diagram files, at a fraction of the cost of a commercial CASE tool. Of course, the actual power of such a hybrid (since Dia is just a diagramming tool) would only be a fraction of the power of a high-quality CASE tool, but it would be a great step forward.

Reused code

The simple_input_parser and error_log classes are entirely reused, as is single_variable_function.

number_array_list

/*
*/

#ifndef NUMBER_ARRAY_LIST_H
#define NUMBER_ARRAY_LIST_H

#include <list>
#include <vector>

#ifndef SINGLE_VARIABLE_FUNCTION_H
#include "single_variable_function.h"
#endif

class number_array_list : public std::list< std::vector< double > >
{
 protected:
  int m_field_count;

 public:
  int field_count( void ) const;
  std::vector< double > head( void ) const;
  number_array_list tail( void ) const;
  number_array_list suffixed_by_item( const std::vector< double >& entry ) const;
  number_array_list suffixed_by_list( const number_array_list& rhs ) const;
  number_array_list mapped_to( int field_index, const single_variable_function& f ) const;
  number_array_list multiplied_by_list( int field_index, const number_array_list& rhs ) const;
  number_array_list sorted_by( int field_index ) const;
  number_array_list items_less_than( int field_index, double value ) const;
  number_array_list items_greater_than_or_equal_to( int field_index, double value ) const;

  bool is_valid_entry( const std::vector< double >& entry ) const;
  bool is_valid_field_index( int field_index ) const;
  bool is_sorted_by( int field_index ) const;
  double sum_by_field( int field_index ) const;
  double mean_by_field( int field_index ) const;
  int entry_count( void ) const;
  
  void set_field_count( int new_field_count );
  void add_entry( const std::vector< double >& entry );

  void make_from_entry( const std::vector< double >& entry );
  void make( void );
  void make_from_list( const number_array_list& rhs );
  number_array_list( void );
};

#endif

/*
*/
/*
*/

#include "number_array_list.h"

#ifndef CONTRACT_H
#include "contract.h"
#endif

int
number_array_list::field_count( void ) const
{
  return m_field_count;
}

std::vector< double >
number_array_list::head( void ) const
{
  REQUIRE( entry_count() >= 1 );
  return *(begin());
}

number_array_list
number_array_list::tail( void ) const
{
  number_array_list Result;
  if ( entry_count() > 1 )
    {
      number_array_list::const_iterator tail_head = begin();
      ++tail_head;
      Result.insert( Result.begin(), tail_head, end() );
      Result.m_field_count = field_count();
      ENSURE( Result.entry_count() == entry_count() - 1 );
    }
  return Result;
}

number_array_list
number_array_list::suffixed_by_item( const std::vector< double >& entry ) const
{
  number_array_list Result;
  Result.make_from_list( *this );
  Result.add_entry( entry );
  ENSURE( Result.entry_count() == entry_count() + 1 );
  return Result;
}

number_array_list
number_array_list::suffixed_by_list( const number_array_list& rhs ) const
{
  number_array_list Result;
  Result.make_from_list( *this );
  for ( number_array_list::const_iterator iter = rhs.begin(); iter != rhs.end(); ++iter )
    {
      Result.add_entry( *iter );
    }
  ENSURE( Result.entry_count() == entry_count() + rhs.entry_count() );
  return Result;
}

number_array_list
number_array_list::mapped_to( int field_index, const single_variable_function& f ) const
{
  REQUIRE( is_valid_field_index( field_index ) );
  number_array_list Result;
  if ( entry_count() > 0 )
    {
      std::vector<double> new_entry = head();
      new_entry[ field_index ] = f.at( head()[ field_index ] );
      Result.make_from_list( Result.suffixed_by_item( new_entry ).suffixed_by_list( tail().mapped_to( field_index, f )));
    }
  ENSURE( Result.entry_count() == entry_count() );
  return Result;
}

number_array_list
number_array_list::multiplied_by_list( int field_index, const number_array_list& rhs ) const
{
  REQUIRE( entry_count() == rhs.entry_count() );
  REQUIRE( field_count() == rhs.field_count() );
  REQUIRE( is_valid_field_index( field_index ) );
  number_array_list Result;
  if ( entry_count() > 0 )
    {
      std::vector< double > new_entry = head();
      new_entry[ field_index ] = head()[ field_index ] * rhs.head()[field_index];
      Result.make_from_list( Result.suffixed_by_item( new_entry ).
			      suffixed_by_list( tail().multiplied_by_list( field_index, rhs ) ) );
    }
  ENSURE( Result.entry_count() == entry_count() );
  return Result;
}

number_array_list
number_array_list::sorted_by( int field_index ) const
{
  REQUIRE( is_valid_field_index( field_index ) );
  number_array_list Result;
  if ( is_sorted_by( field_index ) )
    {
      Result.make_from_list( *this );
    }
  else
    {
      Result.make_from_list( tail().items_less_than( field_index, head()[field_index] ).sorted_by( field_index ).
			     suffixed_by_item( head() ).
			     suffixed_by_list( tail().items_greater_than_or_equal_to( field_index, head()[field_index] ).sorted_by( field_index )));
    }
  ENSURE( Result.entry_count() == entry_count() );
  return Result;
}

number_array_list
number_array_list::items_less_than( int field_index, double value ) const
{
  REQUIRE( is_valid_field_index( field_index ) );
  number_array_list Result;
  for ( number_array_list::const_iterator iter = begin(); iter != end(); ++iter )
    {
      if ( (*iter)[field_index] < value )
	{
	  Result.add_entry( *iter );
	}
    }
  ENSURE( Result.entry_count() <= entry_count() );
  return Result;
}

number_array_list
number_array_list::items_greater_than_or_equal_to( int field_index, double value ) const
{
  REQUIRE( is_valid_field_index( field_index ) );
  number_array_list Result;
  for ( number_array_list::const_iterator iter = begin(); iter != end(); ++iter )
    {
      if ( (*iter)[field_index] >= value )
	{
	  Result.add_entry(*iter);
	}
    }
  ENSURE( Result.entry_count() <= entry_count() );
  return Result;
}

bool
number_array_list::is_valid_entry( const std::vector<double>& entry ) const
{
  bool Result = false;
  if ( ( entry_count() == 0 ) 
       || ( ( entry_count() > 0 ) && ( entry.size() == field_count() ) ) )
    {
      Result = true;
    }
  return Result;
}

bool
number_array_list::is_valid_field_index( int field_index ) const
{
  bool Result = true;
  if ( entry_count() > 0 )
    {
      Result = ( 0 <= field_index ) && ( field_index < m_field_count );
    }
  return Result;
}

bool
number_array_list::is_sorted_by( int field_index ) const
{
  REQUIRE( is_valid_field_index( field_index ) );
  bool Result = true;
  if ( entry_count() > 1 )
    {
      Result = ( head()[field_index] < tail().head()[field_index] ) && tail().is_sorted_by( field_index );
    }
  return Result;
}

double
number_array_list::sum_by_field( int field_index ) const
{
  REQUIRE( is_valid_field_index( field_index ) )
  double Result = 0;
  if ( entry_count() > 0 )
    {
      Result = head()[field_index] + tail().sum_by_field( field_index );
    }
  return Result;
}

double
number_array_list::mean_by_field( int field_index ) const
{
  REQUIRE( is_valid_field_index( field_index ) );
  REQUIRE( entry_count() > 0 );
  double Result = sum_by_field( field_index ) / static_cast<double>( entry_count() );
  return Result;
}

int
number_array_list::entry_count( void ) const
{
  return size();
}

void
number_array_list::set_field_count( int new_field_count )
{
  REQUIRE( entry_count() == 0 );
  m_field_count = new_field_count;
  ENSURE( field_count() == new_field_count );
}

void
number_array_list::add_entry( const std::vector< double >& entry )
{
  if ( entry_count() == 0 )
    {
      set_field_count( entry.size() );
    }
  REQUIRE( is_valid_entry( entry ) );
  const int old_entry_count = entry_count();
  push_back( entry );
  ENSURE( entry_count() == old_entry_count + 1 );
}

void
number_array_list::make_from_entry( const std::vector<double>& entry )
{
  make();
  add_entry( entry );
  ENSURE( entry_count() == 1 );
  ENSURE( head() == entry );
}

void
number_array_list::make( void )
{
  clear();
  m_field_count = -1;
  ENSURE( m_field_count == -1 );
  ENSURE( entry_count() == 0 );
}

void
number_array_list::make_from_list( const number_array_list& rhs )
{
  make();
  insert( begin(), rhs.begin(), rhs.end() );
  m_field_count = rhs.field_count();
  ENSURE( entry_count() == rhs.entry_count() );
}

number_array_list::number_array_list( void )
{
  make();
}

/*
*/

number_array_list_parser

#ifndef NUMBER_ARRAY_LIST_PARSER_H
#define NUMBER_ARRAY_LIST_PARSER_H

#ifndef SIMPLE_INPUT_PARSER_H
#include "simple_input_parser.h"
#endif
#ifndef NUMBER_ARRAY_LIST_H
#include "number_array_list.h"
#endif

class number_array_list_parser: public simple_input_parser
{
 protected:
  number_array_list number_list;
  enum state_t { Parsing_historical_data, Parsing_prediction_data };
  state_t state;

 public:
  virtual std::string transformed_line (const std::string & line) const;
  std::string string_stripped_of_comments (const std::string & str) const;
  static bool is_double (const std::string & str);
  static double double_from_string (const std::string & str);
  static const std::string & historical_data_terminator;
  static const std::string & inline_comment_begin;
  bool last_line_is_blank (void);

  virtual void parse_last_line (void);
  void parse_last_line_as_historical_data (void);
  void parse_last_line_as_end_of_historical_data (void);
  void make( void );
  number_array_list_parser( void );

  bool last_line_is_valid_historical_data( void ) const;
  static std::vector< std::string > split_string( const std::string& string_to_split, const std::string& separator );
  static std::string string_before_separator( const std::string& string_to_split, const std::string& separator );
  static std::string string_after_separator( const std::string& string_to_split, const std::string& separator );
  static std::vector< double > numbers_from_string( const std::string& string_to_split );

  void print_number_list_sorted_by( int field_index ) const;
  void print_entry( const std::vector< double >& entry ) const;
};

#endif
/*
*/

#include "number_array_list_parser.h"
#ifndef WHITESPACE_STRIPPER_H
#include "whitespace_stripper.h"
#endif
#ifndef ERROR_LOG_H
#include "error_log.h"
#endif
#ifndef CONTRACT_H
#include "contract.h"
#endif

void
number_array_list_parser::make (void)
{
  simple_input_parser::reset ();
  state = Parsing_historical_data;
  number_list.make ();
}

std::string number_array_list_parser::transformed_line (const std::string & str) const
{
  return whitespace_stripper::string_stripped_of_whitespace (string_stripped_of_comments (str));
}

std::string
  number_array_list_parser::string_stripped_of_comments (const std::string & str) const
{
  const std::string::size_type comment_index = str.find (inline_comment_begin);
  return str.substr (0, comment_index);
}

bool 
number_array_list_parser::is_double (const std::string & str)
{
  bool
    Result = true;
  char *
    conversion_end = NULL;
  strtod (str.c_str (), &conversion_end);
  if (conversion_end == str.data ())
    {
      Result = false;
    }
  return Result;
}

double
number_array_list_parser::double_from_string (const std::string & str)
{
  REQUIRE (is_double (str));
  return strtod (str.c_str (), NULL);
}


const
  std::string & number_array_list_parser::historical_data_terminator = "stop";

const
  std::string & number_array_list_parser::inline_comment_begin = "--";

bool number_array_list_parser::last_line_is_blank (void)
{
  if (last_line ().length () == 0)
    {
      return true;
    }
  else
    {
      return false;
    }
}

void
number_array_list_parser::parse_last_line (void)
{
  if (last_line_is_blank ())
    {
      return;
    }
  if ( state == Parsing_historical_data )
    {
      if ( last_line () == historical_data_terminator)
	{
	  parse_last_line_as_end_of_historical_data ();
	}
      else
	{
	  parse_last_line_as_historical_data ();
	}
      //        else
      //  	{
      //  	  parse_last_line_as_prediction ();
      //  	}
    }
}


void
number_array_list_parser::parse_last_line_as_historical_data (void)
{
  if ( last_line_is_valid_historical_data() )
    {
      const std::vector< double >this_entry = numbers_from_string( last_line() );
      number_list.add_entry( this_entry );
    }
  else
    {
      error_log errlog;
      errlog.log_error( std::string( "Invalid data entry: " ) + last_line() );
    }
}
  
void
number_array_list_parser::parse_last_line_as_end_of_historical_data (void)
{
  REQUIRE (last_line () == historical_data_terminator);
  cout << "Historical data read; " 
       << number_list.entry_count() << " entries, " 
       << number_list.field_count() << " fields.\n";
  if ( number_list.entry_count() > 0 )
    {
      for ( int i = 0; i < number_list.field_count(); ++i )
	{
	  print_number_list_sorted_by( i );
	}
    }
  state = Parsing_prediction_data;
}

number_array_list_parser::number_array_list_parser (void)
{
  make();
}


bool
number_array_list_parser::last_line_is_valid_historical_data( void ) const
{
  const std::vector< std::string > substrings = split_string( last_line(), "," );
  bool Result = true;
  //make sure we have a valid field count for the number_array_list
  if ( ( number_list.entry_count() > 0 ) && ( substrings.size() != number_list.field_count() ) )
    {
      Result = false;
    }
  //...and that each substring can be interpreted as a double
  for ( int i = 0; i < substrings.size(); ++i )
    {
      if ( !is_double( substrings[ i ] ) )
	{
	  Result = false;
	}
    }
  return Result;
}

std::vector< std::string >
number_array_list_parser::split_string( const std::string& string_to_split, const std::string& separator ) 
{
  std::vector< std::string > Result;
  const std::string prefix = string_before_separator( string_to_split, separator );
  const std::string remainder = string_after_separator( string_to_split, separator );
  Result.push_back( prefix );
  if ( remainder.size() > 0 )
    {
      const std::vector< std::string > split_remainder = split_string( remainder, separator );
      Result.insert( Result.end(), split_remainder.begin(), split_remainder.end() );
    }
  return Result;
}

std::string
number_array_list_parser::string_before_separator( const std::string& string_to_split, const std::string& separator )
{
  const std::string::size_type separator_position = string_to_split.find( separator );
  std::string Result;
  if ( separator_position == string_to_split.npos )
    {
    //not found; result is entire string
      Result = string_to_split;
    }
  else
    {
      Result = string_to_split.substr( 0, separator_position );
    }
  return Result;
}

std::string
number_array_list_parser::string_after_separator( const std::string& string_to_split, const std::string& separator )
{
  const std::string::size_type separator_position = string_to_split.find( separator );
  std::string Result;
  if ( separator_position == string_to_split.npos )
    {
      //not found; result is empty
      Result = "";
    }
  else
    {
      Result = string_to_split.substr( separator_position + separator.size(), string_to_split.size() );
    }
  return Result;
}

std::vector< double >
number_array_list_parser::numbers_from_string( const std::string& string_to_split )
{
  const std::vector< std::string > number_strings = split_string( string_to_split, "," );
  std::vector< double > Result;
  for ( std::vector< std::string >::const_iterator iter = number_strings.begin(); iter != number_strings.end(); ++iter )
    {
      CHECK( is_double( *iter ) );
      const double new_value = double_from_string( *iter );
      Result.push_back( new_value );
    }
  return Result;
}

void
number_array_list_parser::print_number_list_sorted_by( int field_index ) const
{
  cout << "Sorted by field " << field_index << "\n";
  const number_array_list sorted_list = number_list.sorted_by( field_index );
  for ( number_array_list::const_iterator iter = sorted_list.begin(); iter != sorted_list.end(); ++iter )
    {
      print_entry( *iter );
      cout << "\n";
    }
  cout << "\n\n";
}

void 
number_array_list_parser::print_entry( const std::vector< double >& entry ) const
{
  cout << "( ";
  for ( std::vector< double >::const_iterator iter = entry.begin(); iter != entry.end(); ++iter )
    {
      cout << *iter;
      if ( ( iter + 1 ) != entry.end() )
	{
	  cout << ", ";
	}
    }
  cout << " )";
}


/*
*/

main.cpp

/*
*/

#include <fstream>
#include <iostream>
#include "string.h"

#ifndef NUMBER_ARRAY_LIST_PARSER_H
#include "number_array_list_parser.h"
#endif
#ifndef CONTRACT_H
#include "contract.h"
#endif

istream *
input_stream_from_args (int arg_count, const char **arg_vector)
{
  istream *Result = NULL;
  if (arg_count == 1)
    {
      Result = &cin;
    }
  else
    {
      const char *help_text =
	"PSP exercise 8A: Sort a linked list of entries.\nUsage:\n\tpsp_5a\n\n";
      cout << help_text;
    }
  return Result;
}

int
main (int arg_count, const char **arg_vector)
{
  //get the input stream, or print the help text as appropriate
  istream *input_stream = input_stream_from_args (arg_count, arg_vector);
  if (input_stream != NULL)
    {
      number_array_list_parser parser;
      parser.set_input_stream (input_stream);
      try 
	{
	  parser.parse_until_eof ();
	}
      catch ( yak_exception& e )
	{
	  cerr << "Exception: " << e.what() << "\n";
	}
    }
}

/*
*/

number_array_list.e

class NUMBER_ARRAY_LIST
   
inherit
   LINKED_LIST[ ARRAY[ DOUBLE ] ]
      redefine
	 make;
   
creation
   make, make_from_entry, make_from_list
      
feature { ANY }
   
   field_count : INTEGER
	 --number of fields allowed in an entry
   
   head : like item is
      --first item
      require
	 entry_count >= 1
      do
	 Result := first;
      end
   
   tail : like Current is
	 --all items after the first
      do
	 !!Result.make
	 if ( entry_count > 1 ) then
	    Result := slice( lower + 1, upper );
	    Result.set_field_count( field_count )
	 end --if
      ensure
	 ( entry_count > 1 ) implies Result.entry_count = entry_count - 1 and Result.field_count = field_count
      end
   
   suffixed_by_list( rhs: like Current ) : like Current is
	 --the list, suffixed by another list
      local
	 i : INTEGER
      do
	 !!Result.make_from_list( Current );
	 from
	    i := rhs.lower
	 until
	    not rhs.valid_index ( i )
	 loop
	    Result.add_entry( rhs.item( i ) );
	    i := i + 1
	 end --from
      ensure
	 Result.entry_count = entry_count + rhs.entry_count
      end
   
   make_from_entry( entry : like item ) is
	 --clear the list, then add the entry
      do
	 make
	 add_entry( entry )
      ensure
	 head.equals( entry )
	 field_count = entry.count
      end
   
   make is
	 --clear the list
      do
	 Precursor
	 field_count := -1
      ensure
	 entry_count = 0
	 field_count = -1
      end
   
   make_from_list( rhs: like Current ) is
	 --clear the list, setting it equal to another list
      do
	 from_collection( rhs )
	 field_count := rhs.field_count
      end
   
   sum_by_field( field_index : INTEGER ) : DOUBLE is
	 --the sum of a given field over all entries
      require
	 is_valid_field_index( field_index )
      do
	 if entry_count = 0 then
	    Result := 0
	 else
	    Result := head.item( field_index ) + tail.sum_by_field( field_index )
	 end
      end
   
   mean_by_field( field_index : INTEGER ) : DOUBLE is
	 --the mean of a given field over all entries
      require
	 is_valid_field_index( field_index )
	 entry_count >= 1
      do
	 Result := sum_by_field( field_index ) / entry_count.to_double
      end
   
   entry_count : INTEGER is
	 --the number of entries
      do
	 Result := count
      end
   
   add_entry( new_entry : like item ) is
	 -- adds an entry to the end of the list
      require
	 is_valid_entry( new_entry )
      do
	 if entry_count = 0 then
	    set_field_count( new_entry.count )
	 end
	 add_last( new_entry )
      ensure
	 entry_count = old entry_count + 1
      end
   
   mapped_to( field_index : INTEGER; f: SINGLE_VARIABLE_FUNCTION ) : like Current is
	 --the elements, with the given field mapped to f
      require
	 is_valid_field_index( field_index )
      local
	 new_entry : like item
      do
	 !!Result.make
	 if entry_count > 0 then
	    new_entry := head
	    new_entry.put( f.at( head.item(field_index) ), field_index )
	    Result.add_entry( new_entry );
	    Result := Result.suffixed_by_list( tail.mapped_to( field_index, f ) )
	 end -- if
      ensure
	 Result.entry_count = entry_count
      end
   
   multiplied_by_list( field_index : INTEGER; rhs : like Current ) : like Current is
      --the elements, with the given field multiplied by the 
	 --corresponding field in rhs
      require
	 entry_count = rhs.entry_count
	 field_count = rhs.field_count
	 is_valid_field_index( field_index )
      local
	 new_entry : like item
      do
	 !!Result.make
	 if entry_count > 0 then
	    new_entry := head
	    new_entry.put( head.item( field_index ) * rhs.head.item( field_index ), field_index )
	    Result.add_entry( new_entry )
	    Result := Result.suffixed_by_list( tail.multiplied_by_list( rhs.tail ) )
	 end
      ensure
	 Result.entry_count = entry_count
      end
   
   sorted_by( field_index : INTEGER ) : like Current is
	 --the list, sorted by the given field
      require
	 is_valid_field_index( field_index )
      do
	 if is_sorted_by( field_index ) then
	    !!Result.make_from_list( Current )
	 else
	    !!Result.make_from_list( tail.items_less_than( field_index, head.item( field_index ) ).sorted_by( field_index ).
				     suffixed_by_item( head ).
				     suffixed_by_list( tail.items_greater_than_or_equal_to( field_index, head.item( field_index ) ).sorted_by( field_index ) ) )
	 end -- if
      ensure
	 Result.entry_count = entry_count
      end
   
   is_valid_entry( entry : like item ) : BOOLEAN is
	 --whether entry is a valid entry
      do
	 if ( entry_count = 0 and entry.count > 0 ) or ( entry.count = field_count and entry.lower = 1 ) then
	    Result := true
	 else
	    Result := false
	 end
      end
   
   items_less_than( field_index : INTEGER; value : DOUBLE ) : like Current is
	 --list of items less than the given value in the given field
      require
	 is_valid_field_index( field_index )
      local
	 i : INTEGER
      do
	 !!Result.make
	 from
	    i := lower
	 until
	    not valid_index( i )
	 loop
	    if not( item( i ).item( field_index ) >= value ) then
	       Result.add_entry( item( i ) )
	    end
	    i := i + 1
	 end
      ensure
	 Result.entry_count <= entry_count
      end
   
   items_greater_than_or_equal_to( field_index : INTEGER; value : DOUBLE ) : like Current is
      --list of items greater than or equal to the given value in the 
	 --given field
      require
	 is_valid_field_index( field_index )
      local
	 i : INTEGER
      do
	 !!Result.make
	 from
	    i := lower
	 until
	    not valid_index( i )
	 loop
	    if item(i).item( field_index ) >= value then
	       Result.add_entry( item( i ) )
	    end
	    i := i + 1
	 end
      ensure
	 Result.entry_count <= entry_count
      end
   
   suffixed_by_item( entry : like item ) : like Current is
	 --the list, suffixed by a single item
      require
	 is_valid_entry( entry )
      do
	 !!Result.make_from_list( Current )
	 Result.add_entry( entry )
      ensure
	 Result.entry_count = entry_count + 1
      end
   
   set_field_count( new_field_count : INTEGER ) is
	 --sets the field count
      require
	 entry_count = 0 or ( entry_count > 0 implies head.count = new_field_count )
      do
	 field_count := new_field_count
      ensure
	 field_count = new_field_count
      end
   
   is_valid_field_index( field_index : INTEGER ) : BOOLEAN is
	 --whether the given field index is valid
      do
	 if entry_count = 0 then
	    Result := true
	 else
	    Result := ( ( 1 <= field_index ) and ( field_index <= field_count ) )
	 end
      end
   
   is_sorted_by( field_index : INTEGER ) : BOOLEAN is
	 --whether the list is sorted by the given field index
      require
	 is_valid_field_index( field_index )
      do
	 Result := true
	 if entry_count > 1 then
	    Result := ( head.item( field_index ) < tail.head.item( field_index ) ) 
	       and tail.is_sorted_by( field_index )
	 end
      end
   
end

	 
			     
	 

number_array_list_parser.e

class NUMBER_ARRAY_LIST_PARSER
--reads a list of number pairs, and performs linear regression analysis

inherit 
   SIMPLE_INPUT_PARSER
      redefine parse_last_line, transformed_line
      end; 
   
creation {ANY} 
   make

feature {ANY} 
   
   inline_comment_begin: STRING is "--";
   
   string_stripped_of_comment(string: STRING): STRING is 
      --strip the string of any comment
      local 
         comment_index: INTEGER;
      do  
         if string.has_string(inline_comment_begin) then 
            comment_index := string.index_of_string(inline_comment_begin);
            if comment_index = 1 then 
               !!Result.make_from_string( "" );
            else 
               Result := string.substring(1,comment_index - 1);
            end; 
         else 
            Result := string.twin;
         end; 
      end -- string_stripped_of_comment
   
   string_stripped_of_whitespace(string: STRING): STRING is 
      --strip string of whitespace
      do  
         Result := string.twin;
         Result.left_adjust;
         Result.right_adjust;
      end -- string_stripped_of_whitespace
   
   transformed_line(string: STRING): STRING is 
      --strip comments and whitespace from parseable line      
      do  
         Result := string_stripped_of_whitespace(string_stripped_of_comment(string));
      end -- transformed_line
   
   number_list: NUMBER_ARRAY_LIST;
   
   state : INTEGER
   
   Parsing_historical_data : INTEGER is unique
   
   Parsing_prediction_data : INTEGER is unique
   
   historical_data_terminator: STRING is "stop";
   
   double_from_string(string: STRING): DOUBLE is 
      require 
         string.is_double or string.is_integer; 
      do  
         if string.is_double then 
            Result := string.to_double;
         elseif string.is_integer then 
            Result := string.to_integer.to_double;
         end; 
      end -- double_from_string
   

feature {ANY} --parsing

   reset is 
      --resets the parser and makes it ready to go again
      do  
         state := Parsing_historical_data;
         number_list.make;
      end -- reset
   
   make is 
      do  
         !!number_list.make;
         reset;
      end -- make
   
   parse_last_line_as_historical_data is 
      --interpret last_line as a pair of comma-separated values
      local 
         error_log: ERROR_LOG;
	 this_line_numbers : ARRAY[ DOUBLE ]
      do  
	 !!error_log.make
	 if last_line_is_valid_historical_data then
	    this_line_numbers := numbers_from_string( last_line )
	    number_list.add_entry( this_line_numbers )
	 else
	    error_log.log_error( "Invalid historical data: " + last_line + "%N" )
	 end
      end
   
   parse_last_line_as_end_of_historical_data is 
      --interpret last line as the end of historical data
      require 
         last_line.compare(historical_data_terminator) = 0; 
      local
	 i : INTEGER
      do  
	 state := Parsing_prediction_data
	 from
	    i := 1
	 until
	    not number_list.is_valid_field_index( i )
	 loop
	    print_number_list_sorted_by( i )
	    i := i + 1
	 end
      end -- parse_last_line_as_end_of_historical_data
   
   parse_last_line is 
      --parse the last line according to state
      do  
         if not last_line.empty then 
	    if state = Parsing_historical_data then
	       if last_line.compare(historical_data_terminator) = 0 then 
		  parse_last_line_as_end_of_historical_data;
	       else 
                  parse_last_line_as_historical_data;
               end; 
            end; 
         end; 
      end -- parse_last_line
   
   last_line_is_valid_historical_data : BOOLEAN is
	 --whether last line is valid historical data
      local
	 substrings : ARRAY[ STRING ]
	 i : INTEGER
      do
	 substrings := split_string( last_line, "," )
	 Result := true
	 if ( number_list.entry_count > 0 and substrings.count /= number_list.field_count ) then
	    Result := false;
	 end
	 --check and see if each substring is convertible to a double
	 from
	    i := substrings.lower
	 until
	    not substrings.valid_index( i )
	 loop
	    if not ( substrings.item(i).is_double or substrings.item(i).is_integer ) then
	       Result := false;
	    end
	    i := i + 1
	 end
      end
   
   split_string( string_to_split, separator : STRING ) : ARRAY[ STRING ] is
      --a list of components of a string, separated by the given 
      --separator, ie split_string( "1,2,3", "," ) = [ "1", "2", "3" ]
      local
	 prior_to_separator : STRING
	 remainder : STRING
	 split_remainder : ARRAY[ STRING ]
	 i : INTEGER
      do
	 prior_to_separator := string_before_separator( string_to_split, separator )
	 remainder := string_after_separator( string_to_split, separator )
	 !!Result.make( 1, 0 )
	 Result.add_last( prior_to_separator )
	 if remainder.count > 0 then
	    split_remainder := split_string( remainder, separator )
	    from
	       i := split_remainder.lower
	    until
	       not split_remainder.valid_index( i )
	    loop
	       Result.add_last( split_remainder.item( i ) )
	       i := i + 1
	    end
	 end
      end
   
   string_before_separator( string_to_split, separator : STRING ) : STRING is
	 --the part of a string which comes before the separator, or 
	 --the whole string if it's not found
      local
	 separator_index : INTEGER
      do
	 separator_index := string_to_split.substring_index( separator, 1 )
	 if ( separator_index = 0 ) then
	    --not found; copy whole string
	    Result := string_to_split.twin
	 else
	    Result := string_to_split.substring( 1, separator_index - 1 )
	 end
      end
   
   string_after_separator( string_to_split, separator : STRING ) : STRING is
	 --the part of the string after the separator,
      local
	 separator_index : INTEGER
      do
	 separator_index := string_to_split.substring_index( separator, 1 )
	 if ( separator_index = 0 ) then
	    --not found; result is empty
	    !!Result.make_from_string( "" )
	 else
	    Result := string_to_split.substring( separator_index + separator.count, string_to_split.count )
	 end
      end
   
   numbers_from_string( string_to_split : STRING ) : ARRAY[ DOUBLE ] is
	 --an array of numbers, from a string of numbers separated by commas
      local
	 number_strings : ARRAY[ STRING ]
	 i : INTEGER
      do
	 !!Result.make( 1, 0 )
	 number_strings := split_string( string_to_split, "," )
	 from
	    i := number_strings.lower
	 until
	    not number_strings.valid_index( i )
	 loop
	    check
	       number_strings.item(i).is_double or number_strings.item(i).is_integer
	    end
	    Result.add_last( double_from_string( number_strings.item( i ) ) )
	    i := i + 1
	 end
      end
   
   print_number_list_sorted_by( field_index : INTEGER ) is
	 --prints the number list, sorted by field
      local
	 sorted_list : NUMBER_ARRAY_LIST
	 i : INTEGER
      do
	 sorted_list := number_list.sorted_by( field_index )
	 std_output.put_string( "Sorted by " )
	 std_output.put_integer( field_index )
	 std_output.put_new_line
	 from
	    i := sorted_list.lower
	 until
	    not sorted_list.valid_index( i )
	 loop
	    print_entry( sorted_list.item( i ) )
	    std_output.put_string( "%N" )
	    i := i + 1
	 end
	 std_output.put_string( "%N%N" )
      end
   
   print_entry( entry : ARRAY[ DOUBLE ] ) is
	 --prints an entry
      local
	 i : INTEGER
      do
	 std_output.put_string( "( " )
	 from
	    i := entry.lower
	 until
	    not entry.valid_index( i )
	 loop
	    std_output.put_double( entry.item( i ) )
	    if ( entry.valid_index( i + 1 ) ) then
	       std_output.put_string( ", " )
	    end
	    i := i + 1
	 end
	 std_output.put_string( " )" )
      end

end -- class NUMBER_ARRAY_LIST_PARSER

main.e

class MAIN

creation {ANY} 
   make

feature {ANY} 
   
   make is 
      local 
         parser: NUMBER_ARRAY_LIST_PARSER;
      do  
         !!parser.make;
         parser.set_input(io);
         parser.parse_until_eof;
      end -- make

end -- MAIN

Code Review

Reviewing code is hard! It takes a great deal of time, and I keep missing obvious errors, which shows that I need to spend more attention to details. I'm not convinced yet that reviewing code prior to compile is more efficient-- at least not in the short run. Hopefully, I'm developing a habit of attention to detail which will pay dividends in the long run.

Compile

As expected, compilation caught several very simple errors which somehow eluded both of my reviews. This is somewhat depressing-- but compilation only caught about 36% of my errors, down from my overall average of 45%, so those reviews are doing something!

Test

Several errors escaped until test. I'm not pleased about this, but on the other hand, only 12% of errors remained at test-- my previous average, before adding design and code reviews, was about 28%, so we're seeing roughly half the errors in test now-- and that's a significant improvement. Most of the errors caught in test were fairly easy to identify and fix. I'll omit the test output, because it only printed the lists, sorted-- and that's fairly easy to visualize.

Postmortem

Twice now, inspections have caught significant numbers of defects, and reduced the number introduced during test, yet kept my LOC/hour productivity about the same. On the surface, it would appear that they don't add much-- after all, productivity is about the same, and the total number of defects introduced (and fixed) hasn't changed.

But I'm reminded of something Humphrey mentions in the book about hidden defects-- the ones that are still there after test, which we haven't run into yet. Fewer defects are making it to test-- and therefore there are most likely fewer left after test, leading to a higher-quality product and less predicted maintenance effort. And that makes the reviews worthwhile, even if productivity never budges (and I'm convinced that it'll get better after I get more used to doing reviews).

PSP2.1 Project Plan Summary

Table 9-2. Project Plan Summary

Student:Victor B. PutzDate:000129
Program:CorrelationProgram#8A
Instructor:WellsLanguage:C++
SummaryPlanActualTo date
Loc/Hour 464546
Planned time174 658
Actual time 393906
CPI (cost/performance index)  0.73
%reused353240
Test Defects/KLOC342031
Total Defects/KLOC137158141
Yield (defects before test/total defects)758778
% Appraisal COQ1.8417.65
% Failure COQ3411.729.88
COQ A/F Ratio0.053231.50.16
Program SizePlanActualTo date
Base1717 
Deleted00 
Modified22 
Added343296 
Reused1921451133
Total New and Changed3452981473
Total LOC5524582848
Total new/reused000
Upper Prediction Interval (70%)489  
Lower Prediction Interval (70%)200  
Time in Phase (min):PlanActualTo DateTo Date%
Planning357537219
Design197924713
Design Review224402
Code459949026
Code Review245573
Compile10181146
Test492845824
Postmortem12251367
Total1743931914100
Total Time UPI (70%)214   
Total Time LPI (70%)132   
Defects Injected ActualTo DateTo Date %
Plan 000
Design15146531
Design Review0000
Code313313766
Code Review0000
Compile1/2032
Test1/2032
Total development4747208~100
Defects Removed ActualTo DateTo Date %
Planning 000
Design 000
Design Review1/2594
Code1084421
Code Review1/211157
Compile23179445
Test1164622
Total development4747208~100
After Development000 
Defect Removal EfficiencyPlanActualTo Date 
Defects/Hour - Design Review1512.513.5 
Defects/Hour - Code Review2014.6715.8 
Defects/Hour - Compile51.456.6749.5 
Defects/Hour - Test612.866 
DRL (design review/test)2.50.972.25 
DRL (code review/test)3.31.142.63 
DRL (compile/test)8.54.48.25 
Eiffel code/compile/test
Time in Phase (min)ActualTo DateTo Date %
Code6530751
Code Review20305
Compile611819
Test3115125
Total122606100
Defects InjectedActualTo DateTo Date %
Design154
Code1911695
Compile000
Test011
Total20122100
Defects RemovedActualTo DateTo Date %
Code011
Code Review91411
Compile67259
Test53529
Total20122100
Defect Removal EfficiencyActualTo Date 
Defects/Hour - Code Review2728 
Defects/Hour - Compile60360 
Defects/Hour - Test1014 
DRL (code review/test)2.72.0 
DRL (compile/test)62.6 

Time Recording Log

Table 9-3. Time Recording Log

Student: Date:000123
  Program: 
StartStopInterruption TimeDelta timePhaseComments
000123 16:30:16000123 17:50:18476plan 
000123 17:52:40000123 19:13:47279design 
000127 20:47:06000128 17:40:45122924design review 
000128 17:55:23000128 19:34:24099code 
000129 09:45:00000129 11:38:586944code review 
000129 11:41:48000129 11:59:37017compile 
000129 11:59:40000129 12:29:10128test 
000129 12:29:35000129 12:54:57025postmortem 
      

Table 9-4. Time Recording Log

Student: Date:000129
  Program: 
StartStopInterruption TimeDelta timePhaseComments
000129 14:57:24000129 16:01:54064code 
000129 16:36:11000129 16:56:34020code review 
000129 16:56:37000129 17:02:4806compile 
000129 17:02:52000129 17:33:34030test 
      

Defect Reporting Logs

Table 9-5. Defect Recording Log

Student: Date:000123
  Program: 
Defect foundTypeReasonPhase InjectedPhase RemovedFix timeComments
000127 20:50:58waigdesigndesign review1sum was not handling empty lists
000127 20:56:29waigdesigndesign review0was not using rhs.tail in multiplied_by_list
000127 20:58:59waigdesigndesign review0can't remember... (logged this late)
000127 21:01:49waigdesigndesign review0wasn't using numbers_from_string in parse_last_line_as_historical_data
000128 17:38:35waigdesigndesign review1changed is_sorted_by to use recursion rather than iteration
000128 18:11:47ctomdesigncode0missed require contract in suffixed_by_list
000128 18:16:10mdomdesigncode1missed the need to add copy operator
000128 18:22:22ctomdesigncode0missed valid_field_index in multiplied_by_list
000128 18:43:50waigdesigncode1forgot to clear before making; also missed entry_count = 1 contract in make_fom_item
000128 18:51:23mdigdesigncode2decided to do away with the field count on a separate line, since it's automatically set by the first entry.
000128 18:58:54mdigdesigncode3forgot to include code to print the numbers sorted by a field!
000128 19:06:19waigdesigncode2wasn't handling blank lines in parse_last_line
000128 19:13:48mdigdesigncode0didn't add code to print the sorted lists after the end of historical data
000129 09:47:37wncmcodecode review0used std::array instead of std::vector
000129 09:49:54wncmcodecode review1was using = instead of make_from_list
000129 10:01:19wccmdesigncode review0was not counting any entry as valid if entry_count = 0
000129 10:05:06wncmcodecode review0was #including the wrong file for the cpp file
000129 10:07:34sytycodecode review0misplaced }
000129 10:08:20sycmcodecode review0was declaring prefix and remainder twice in split_string
000129 11:13:33sytycodecode review0didn't put parentheses around argument for push_back
000129 11:14:56sycmcodecode review1added const to several temporary variables
000129 11:18:22sytycodecode review0missed parentheses on call to field_count
000129 11:30:25waigcodecode review1rewrote logic for split_string to use recursion rather than looping
000129 11:33:31waigcodecode review1wasn't properly handling "separator not found" conditions in before/after separator tests
000129 11:42:16sytycodecompile0missed closing bracket on state enum
000129 11:43:00wncmcodecompile0typed std::array instead of std::vector
000129 11:44:04wttycodecompile0declared parser as type number_array_list instead of number_array_list_parser
000129 11:44:42waigcodecompile0didn't know you couldn't just say begin() + 1 to get the iterator after the start
000129 11:45:57sytycodecompile0used parentheses on data field
000129 11:47:16sytycodecompile0mismatched parentheses
000129 11:48:20wtcmcodecompile0used wrong type for number_array_list::iterator
000129 11:51:00sycmcodecompile0missed parentheses (or added extra ones) accessing data members/no-arg features
000129 11:51:30wncmcodecompile0forgot to rename the predictor_parser elements!
000129 11:52:46syomcodecompile0can't declare static members as static linkage...??
000129 11:53:32wnomcodecompile0renamed numbers to number_list
000129 11:54:08maomcodecompile1forgot to declare errlog
000129 11:55:42iucmcodecompile0did not pass separator as parameter to split_string
000129 11:56:44wncmcodecompile0forgot to prefix size by string_to_split
000129 11:57:26wncmcodecompile0forgot-- it's std::vector, not std::array!
000129 11:58:10wntycodecompile0typed number_string instead of number_strings
000129 11:59:03wntycodecompile0typed number_array_list instead of number_list
000129 12:00:37macmcodetest5forgot to return return value in is_valid_entry
000129 12:08:50macmcodetest6redeclared Result in a local block in string_after_separator-- masked real Result
000129 12:16:15macmcodetest2wasn't assigning field_count in make_from_list
000129 12:22:19macmcodetest2wasn't assigning field_count in tail
000129 12:24:46wncmcodetest1was printing number_list, not sorted_list
000129 12:26:44wacmcodetest2in sorted_by, wasn't sorting the less_than and greater_than_or_equal_to halves!
       

Table 9-6. Defect Recording Log

Student: Date:000129
  Program: 
Defect foundTypeReasonPhase InjectedPhase RemovedFix timeComments
000129 16:38:16maigdesigncode review0forgot to update field_count in make_from_list
000129 16:41:01sytycodecode review0used := instead of = for comparison
000129 16:41:49sytycodecode review0used = instead of := for assignment
000129 16:45:18iuigcodecode review0I don't think you can assign to array.item, so I replaced it with put
000129 16:47:29iuigcodecode review0correctly used make_from_string instead of := for string
000129 16:48:25iuigcodecode review0changed := blurfl to := blurfl.twin in some string operations
000129 16:50:36maigcodecode review0forgot to change assignment from "found_end_of_historical_data" to Parsing_historical_data
000129 16:53:01wncmcodecode review0mistyped number_list.field_count as numbers_field_count
000129 16:53:58macmcodecode review0did not !!Result
000129 16:57:02syigcodecompile0guess you can't say "like feature.subfeature" !
000129 16:57:38sycmcodecompile0forgot "then" after "if"
000129 16:58:12syigcodecompile0can't use "like feature.subfeature"
000129 16:58:49sycmcodecompile0forgot "then" after "if"
000129 16:59:42iuigcodecompile0array.make requires min,max arguments
000129 17:01:18ctigcodecompile0can't use local variables in ensure clause; can't use old in check clause
000129 17:03:37wncmcodetest6was splitting the string with itself, not with the separator!
000129 17:13:57wccmcodetest4problems in is_valid_entry-- didn't count that entry=0 meant that any entry was valid
000129 17:18:48macmcodetest3wasn't setting field_count in tail
000129 17:26:57macmcodetest1was adding items to self instead of Result with suffixed_by_list!
000129 17:29:19wccmcodetest1was not advancing through all possible field indices for sorting