Lecture Notes for Computer Algorithms I

16 September 2003 - Code Review for Assignment 1a

Outline

• Problem

• Design

• Tests

• Analysis

  • Dynamic storage is not your friend.

  • C vs. C++ storage management.

  • Don't Get Cute

  • Assumptions vs. Questions.

  • No prompting or labeling.

Problem

• Most people understood the problem, but there were a few sticking points.

• Printing an error message on failures.

  From the assignment:

  If the program is run with zero or more than one command-line argument, it should print an informative error message and exit; it should to the same if any errors occur while getting the document.

• Freeing dynamic memory.

  From get.h:

  The storage chunk referenced by document should be freed by the caller.

Design

• The design was straightforward, thanks to wishful thinking.

  main()
  
    // check for a command line argument.
  
    pull the url
  
    if there was an error
      print an error message
    else
      print the url resource
  

• Some people, however, got a little too cute.

Tests

• There were four tests:

  1. No command-line argument.

  2. Simple successful get test.

  3. Simple failed get test.

  4. Simple non-text get test.

  • Functional comments, function size, and freeing dynamic memory.

• Test results.

  	c	n	-	+	y
  cmdl	0	2	0	0	19
  get	0	2	0	10	9
  jpeg	0	13	0	4	4
  bad	0	4	0	0	17
  cmnt	0	8	0	0	13
  del	0	16	0	0	5
  size	0	0	0	0	21

Dynamic Storage is Not Your Friend

• Do not use dynamic storage.

  • If you must use it, use it with extreme caution.

• The Dynamic Storage Golden Rule:

  If you allocate it, you free it.

• Calling get() allocates dynamic storage; make sure you free it.

  • Falling off the end of main() is not freeing dynamic storage.

    • It may be a while or perhaps never before control falls off the end of main().

    • People who seem to be abusing dynamic storage are cut almost no slack.

free vs. delete

• new and delete are the dynamic storage routines in C++.

  • There are actually three new procedures in C++.

• malloc() (and friends) and free() are part of C.

  • And so are in, but not of, C++.

  • Favor new and delete over malloc() (and friends) and free() in C++ programs.

• Do not use free() with new.

  • This can cause horrendous programming errors.

  • free() is not C++ aware.

    • It doesn't call destructors, for example.

  my_document.data = new char [size];
  
  free(my_document.data);
  delete [] my_document.data;
  

Don't Get Cute

• Several people made this design optimization:

  
  // get() returns the error string.  If there
  // were errors, the error string is printed
  // and data is empty; otherwise, the error
  // string is empty and data is printed.
  
  std::cout << get(argv[1], page) << "\n";
  std::cout << page.data << "\n";
  

• The relation between the error message and the resource data is not specified.

  • The documentations says only that if size is zero, then so is data.

  • Suppose the error occurred during the middle of the download?

  • And what's with all those newlines?

• Even if correct, when the assumed relation changes, this code breaks.

  • This is known as fragile or brittle code.

Assumptions vs. Questions

• A good assumption is plausible and checkable.

• A plausible assumption is overwhelmingly plausible and supported by evidence.

  • Errors during download is a plausible alternative.

  • No documentation supports either errors or data but not both.

• A checkable assumption should have simple and concise checking code.

  • How would you check the implied relation between error and data?

• An assumption that is not overwhelmingly plausible nor checkable should be a question instead.

  • The get() document should have been clarified.

No Prompting or Labeling

• Please don't prompt for input or excessively label output.

  string tf;
  if ((tf = get(argv[1], doc)) == "")
    cout << "URL Retreived"
         << doc.data
         << endl;
  

• Unasked for output can mess up the tests.

  ***
  *** simple get test
  ***
  
  1c1
  < URL RetreivedHTTP/1.0 200 OK
  ---
  > HTTP/1.0 200 OK
  825d824
  <
  

Points to Remember

• Make sure you understand the problem.

  • Make good assumptions and ask good questions until you do.

• Straightforward and simple designs are best.

  • But not too simple.

• Dynamic storage is not your friend.

  • Don't use it, and use it with extreme caution.

• Don't mix C and C++ dynamic storage.

  • new and delete and malloc() and free() and never the twain shall meet.

• Good assumptions are overwhelmingly plausible and easily checkable.

  • Otherwise, they're questions.

This page last modified on 16 September 2003.