The Personal Software Process: an Independent Study | ||
---|---|---|
Prev | Chapter 9. Lesson 9: Software Design, part I | Next |
Write a program to sort a linked list
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
| |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
--[Humphrey95] |
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.
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.
Using historical data, I get a PROBE-generated estimate of 174 minutes for program 8a.
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).
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.
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.
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.
The simple_input_parser and error_log classes are entirely reused, as is single_variable_function.
/* */ #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(); } /* */ |
#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 << " )"; } /* */ |
/* */ #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"; } } } /* */ |
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 |
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 |
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 |
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.
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!
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.
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).
Table 9-2. Project Plan Summary
Student: | Victor B. Putz | Date: | 000129 |
Program: | Correlation | Program# | 8A |
Instructor: | Wells | Language: | C++ |
Summary | Plan | Actual | To date |
Loc/Hour | 46 | 45 | 46 |
Planned time | 174 | 658 | |
Actual time | 393 | 906 | |
CPI (cost/performance index) | 0.73 | ||
%reused | 35 | 32 | 40 |
Test Defects/KLOC | 34 | 20 | 31 |
Total Defects/KLOC | 137 | 158 | 141 |
Yield (defects before test/total defects) | 75 | 87 | 78 |
% Appraisal COQ | 1.84 | 17.6 | 5 |
% Failure COQ | 34 | 11.7 | 29.88 |
COQ A/F Ratio | 0.05323 | 1.5 | 0.16 |
Program Size | Plan | Actual | To date |
Base | 17 | 17 | |
Deleted | 0 | 0 | |
Modified | 2 | 2 | |
Added | 343 | 296 | |
Reused | 192 | 145 | 1133 |
Total New and Changed | 345 | 298 | 1473 |
Total LOC | 552 | 458 | 2848 |
Total new/reused | 0 | 0 | 0 |
Upper Prediction Interval (70%) | 489 | ||
Lower Prediction Interval (70%) | 200 |
Time in Phase (min): | Plan | Actual | To Date | To Date% |
Planning | 35 | 75 | 372 | 19 |
Design | 19 | 79 | 247 | 13 |
Design Review | 2 | 24 | 40 | 2 |
Code | 45 | 99 | 490 | 26 |
Code Review | 2 | 45 | 57 | 3 |
Compile | 10 | 18 | 114 | 6 |
Test | 49 | 28 | 458 | 24 |
Postmortem | 12 | 25 | 136 | 7 |
Total | 174 | 393 | 1914 | 100 |
Total Time UPI (70%) | 214 | |||
Total Time LPI (70%) | 132 | |||
Defects Injected | Actual | To Date | To Date % | |
Plan | 0 | 0 | 0 | |
Design | 15 | 14 | 65 | 31 |
Design Review | 0 | 0 | 0 | 0 |
Code | 31 | 33 | 137 | 66 |
Code Review | 0 | 0 | 0 | 0 |
Compile | 1/2 | 0 | 3 | 2 |
Test | 1/2 | 0 | 3 | 2 |
Total development | 47 | 47 | 208 | ~100 |
Defects Removed | Actual | To Date | To Date % | |
Planning | 0 | 0 | 0 | |
Design | 0 | 0 | 0 | |
Design Review | 1/2 | 5 | 9 | 4 |
Code | 10 | 8 | 44 | 21 |
Code Review | 1/2 | 11 | 15 | 7 |
Compile | 23 | 17 | 94 | 45 |
Test | 11 | 6 | 46 | 22 |
Total development | 47 | 47 | 208 | ~100 |
After Development | 0 | 0 | 0 | |
Defect Removal Efficiency | Plan | Actual | To Date | |
Defects/Hour - Design Review | 15 | 12.5 | 13.5 | |
Defects/Hour - Code Review | 20 | 14.67 | 15.8 | |
Defects/Hour - Compile | 51.4 | 56.67 | 49.5 | |
Defects/Hour - Test | 6 | 12.86 | 6 | |
DRL (design review/test) | 2.5 | 0.97 | 2.25 | |
DRL (code review/test) | 3.3 | 1.14 | 2.63 | |
DRL (compile/test) | 8.5 | 4.4 | 8.25 |
Eiffel code/compile/test |
Time in Phase (min) | Actual | To Date | To Date % |
Code | 65 | 307 | 51 |
Code Review | 20 | 30 | 5 |
Compile | 6 | 118 | 19 |
Test | 31 | 151 | 25 |
Total | 122 | 606 | 100 |
Defects Injected | Actual | To Date | To Date % |
Design | 1 | 5 | 4 |
Code | 19 | 116 | 95 |
Compile | 0 | 0 | 0 |
Test | 0 | 1 | 1 |
Total | 20 | 122 | 100 |
Defects Removed | Actual | To Date | To Date % |
Code | 0 | 1 | 1 |
Code Review | 9 | 14 | 11 |
Compile | 6 | 72 | 59 |
Test | 5 | 35 | 29 |
Total | 20 | 122 | 100 |
Defect Removal Efficiency | Actual | To Date | |
Defects/Hour - Code Review | 27 | 28 | |
Defects/Hour - Compile | 60 | 360 | |
Defects/Hour - Test | 10 | 14 | |
DRL (code review/test) | 2.7 | 2.0 | |
DRL (compile/test) | 6 | 2.6 |
Table 9-3. Time Recording Log
Student: | Date: | 000123 | |
Program: |
Start | Stop | Interruption Time | Delta time | Phase | Comments |
000123 16:30:16 | 000123 17:50:18 | 4 | 76 | plan | |
000123 17:52:40 | 000123 19:13:47 | 2 | 79 | design | |
000127 20:47:06 | 000128 17:40:45 | 1229 | 24 | design review | |
000128 17:55:23 | 000128 19:34:24 | 0 | 99 | code | |
000129 09:45:00 | 000129 11:38:58 | 69 | 44 | code review | |
000129 11:41:48 | 000129 11:59:37 | 0 | 17 | compile | |
000129 11:59:40 | 000129 12:29:10 | 1 | 28 | test | |
000129 12:29:35 | 000129 12:54:57 | 0 | 25 | postmortem | |
Table 9-4. Time Recording Log
Student: | Date: | 000129 | |
Program: |
Start | Stop | Interruption Time | Delta time | Phase | Comments |
000129 14:57:24 | 000129 16:01:54 | 0 | 64 | code | |
000129 16:36:11 | 000129 16:56:34 | 0 | 20 | code review | |
000129 16:56:37 | 000129 17:02:48 | 0 | 6 | compile | |
000129 17:02:52 | 000129 17:33:34 | 0 | 30 | test | |
Table 9-5. Defect Recording Log
Student: | Date: | 000123 | |
Program: |
Defect found | Type | Reason | Phase Injected | Phase Removed | Fix time | Comments |
000127 20:50:58 | wa | ig | design | design review | 1 | sum was not handling empty lists |
000127 20:56:29 | wa | ig | design | design review | 0 | was not using rhs.tail in multiplied_by_list |
000127 20:58:59 | wa | ig | design | design review | 0 | can't remember... (logged this late) |
000127 21:01:49 | wa | ig | design | design review | 0 | wasn't using numbers_from_string in parse_last_line_as_historical_data |
000128 17:38:35 | wa | ig | design | design review | 1 | changed is_sorted_by to use recursion rather than iteration |
000128 18:11:47 | ct | om | design | code | 0 | missed require contract in suffixed_by_list |
000128 18:16:10 | md | om | design | code | 1 | missed the need to add copy operator |
000128 18:22:22 | ct | om | design | code | 0 | missed valid_field_index in multiplied_by_list |
000128 18:43:50 | wa | ig | design | code | 1 | forgot to clear before making; also missed entry_count = 1 contract in make_fom_item |
000128 18:51:23 | md | ig | design | code | 2 | decided to do away with the field count on a separate line, since it's automatically set by the first entry. |
000128 18:58:54 | md | ig | design | code | 3 | forgot to include code to print the numbers sorted by a field! |
000128 19:06:19 | wa | ig | design | code | 2 | wasn't handling blank lines in parse_last_line |
000128 19:13:48 | md | ig | design | code | 0 | didn't add code to print the sorted lists after the end of historical data |
000129 09:47:37 | wn | cm | code | code review | 0 | used std::array instead of std::vector |
000129 09:49:54 | wn | cm | code | code review | 1 | was using = instead of make_from_list |
000129 10:01:19 | wc | cm | design | code review | 0 | was not counting any entry as valid if entry_count = 0 |
000129 10:05:06 | wn | cm | code | code review | 0 | was #including the wrong file for the cpp file |
000129 10:07:34 | sy | ty | code | code review | 0 | misplaced } |
000129 10:08:20 | sy | cm | code | code review | 0 | was declaring prefix and remainder twice in split_string |
000129 11:13:33 | sy | ty | code | code review | 0 | didn't put parentheses around argument for push_back |
000129 11:14:56 | sy | cm | code | code review | 1 | added const to several temporary variables |
000129 11:18:22 | sy | ty | code | code review | 0 | missed parentheses on call to field_count |
000129 11:30:25 | wa | ig | code | code review | 1 | rewrote logic for split_string to use recursion rather than looping |
000129 11:33:31 | wa | ig | code | code review | 1 | wasn't properly handling "separator not found" conditions in before/after separator tests |
000129 11:42:16 | sy | ty | code | compile | 0 | missed closing bracket on state enum |
000129 11:43:00 | wn | cm | code | compile | 0 | typed std::array instead of std::vector |
000129 11:44:04 | wt | ty | code | compile | 0 | declared parser as type number_array_list instead of number_array_list_parser |
000129 11:44:42 | wa | ig | code | compile | 0 | didn't know you couldn't just say begin() + 1 to get the iterator after the start |
000129 11:45:57 | sy | ty | code | compile | 0 | used parentheses on data field |
000129 11:47:16 | sy | ty | code | compile | 0 | mismatched parentheses |
000129 11:48:20 | wt | cm | code | compile | 0 | used wrong type for number_array_list::iterator |
000129 11:51:00 | sy | cm | code | compile | 0 | missed parentheses (or added extra ones) accessing data members/no-arg features |
000129 11:51:30 | wn | cm | code | compile | 0 | forgot to rename the predictor_parser elements! |
000129 11:52:46 | sy | om | code | compile | 0 | can't declare static members as static linkage...?? |
000129 11:53:32 | wn | om | code | compile | 0 | renamed numbers to number_list |
000129 11:54:08 | ma | om | code | compile | 1 | forgot to declare errlog |
000129 11:55:42 | iu | cm | code | compile | 0 | did not pass separator as parameter to split_string |
000129 11:56:44 | wn | cm | code | compile | 0 | forgot to prefix size by string_to_split |
000129 11:57:26 | wn | cm | code | compile | 0 | forgot-- it's std::vector, not std::array! |
000129 11:58:10 | wn | ty | code | compile | 0 | typed number_string instead of number_strings |
000129 11:59:03 | wn | ty | code | compile | 0 | typed number_array_list instead of number_list |
000129 12:00:37 | ma | cm | code | test | 5 | forgot to return return value in is_valid_entry |
000129 12:08:50 | ma | cm | code | test | 6 | redeclared Result in a local block in string_after_separator-- masked real Result |
000129 12:16:15 | ma | cm | code | test | 2 | wasn't assigning field_count in make_from_list |
000129 12:22:19 | ma | cm | code | test | 2 | wasn't assigning field_count in tail |
000129 12:24:46 | wn | cm | code | test | 1 | was printing number_list, not sorted_list |
000129 12:26:44 | wa | cm | code | test | 2 | in 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 found | Type | Reason | Phase Injected | Phase Removed | Fix time | Comments |
000129 16:38:16 | ma | ig | design | code review | 0 | forgot to update field_count in make_from_list |
000129 16:41:01 | sy | ty | code | code review | 0 | used := instead of = for comparison |
000129 16:41:49 | sy | ty | code | code review | 0 | used = instead of := for assignment |
000129 16:45:18 | iu | ig | code | code review | 0 | I don't think you can assign to array.item, so I replaced it with put |
000129 16:47:29 | iu | ig | code | code review | 0 | correctly used make_from_string instead of := for string |
000129 16:48:25 | iu | ig | code | code review | 0 | changed := blurfl to := blurfl.twin in some string operations |
000129 16:50:36 | ma | ig | code | code review | 0 | forgot to change assignment from "found_end_of_historical_data" to Parsing_historical_data |
000129 16:53:01 | wn | cm | code | code review | 0 | mistyped number_list.field_count as numbers_field_count |
000129 16:53:58 | ma | cm | code | code review | 0 | did not !!Result |
000129 16:57:02 | sy | ig | code | compile | 0 | guess you can't say "like feature.subfeature" ! |
000129 16:57:38 | sy | cm | code | compile | 0 | forgot "then" after "if" |
000129 16:58:12 | sy | ig | code | compile | 0 | can't use "like feature.subfeature" |
000129 16:58:49 | sy | cm | code | compile | 0 | forgot "then" after "if" |
000129 16:59:42 | iu | ig | code | compile | 0 | array.make requires min,max arguments |
000129 17:01:18 | ct | ig | code | compile | 0 | can't use local variables in ensure clause; can't use old in check clause |
000129 17:03:37 | wn | cm | code | test | 6 | was splitting the string with itself, not with the separator! |
000129 17:13:57 | wc | cm | code | test | 4 | problems in is_valid_entry-- didn't count that entry=0 meant that any entry was valid |
000129 17:18:48 | ma | cm | code | test | 3 | wasn't setting field_count in tail |
000129 17:26:57 | ma | cm | code | test | 1 | was adding items to self instead of Result with suffixed_by_list! |
000129 17:29:19 | wc | cm | code | test | 1 | was not advancing through all possible field indices for sorting |