Comments on CS216, Lab 1:

There's some bad things and ugly things in the design of this API:

1. Note the mix of two different naming conventions for the member functions: the use of the underscore vs. mixed-case. E.g. isEmpty() vs. insert_after() in the List class. This is bad practice -- pick one style and stick to it.
2. It's always good to use names that match names that users already know. So List::cardinality() really ought to be named size() since that's the name used in the standard library (e.g. for strings and vectors).
3. The specifications of some of the member functions are incomplete. For example, what is the result of calling List::first() if the list is empty? What's the result of calling List::find() if the parameter is not found in the list?
4. We ask you to write a function that traverses the list in reverse order, but we don't give you a way to access the last element without starting at the beginning. (Unless you make list.tail public.) The best solution to this would be to add a function List::last() the works like first() but return an iterator pointing to the last item in the list.
5. If a call to List::find() fails to find a node matching the parameter value, how does it indicate that the find() did not succeed? This is not specified in the instructions. Solution below.
6. The test-harness code ListTest.cpp has a questionable bit of coding in two places.

   	    ListItr *itr; // done earlier in the code, and then later...
   		itr = &(list->first());

   The call to first() creates a ListItr object and returns it, and normally you assign it to a variable. But not here. The value is created as a temporary variable, not assigned to any variable. But we grab its address and store it in a pointer. That sounds fine, as long as you (the programmer) trusts that the rules of C++ and the particular compiler will keep such temporary variables around. (Do you believe? Clap your hands if you believe!)
   What are the rules about this. Well, frankly, I don't know. What I do know is that this is a really questionable thing to try to be doing, and so let's just not ever do anything like this! It's bad programming, even if it's legal in C++.
   See the updated version of the test-harness file to see an improvement that fixes this.

Updates to the specification for Lab 1 that you may follow if you wish.

1. You may (should) add a member function List::last() to list that creates an ListItr object that references the last node in the list. If you don't have this, it's not clear how you initialize a ListItr object in a way that allows you to start at the end of the list and move backwards. (You can do that in a member function of list, but we ask you to write a print() function outside the List class.
2. What should List::find() return when the parameter is not in the list? Note that find() must return a ListItr object, but for an unsuccessful search that ListItr cannot point to a ListNode that contains list-data. And the client code calling find() must be able to see from this object that the find() didn't succeed. What makes sense here is to return a ListItr object where the current pointer points to the dummy tail-node. This makes sense because you can test the return from find() to see if isPastEnd() is true. So a search might look like the code below:

ListItr itr = list->find(val);
if ( itr.isPastEnd() )
	cout << "Search not successful" << endl;
else
	cout << "Search successful. Found value:" << itr.retrieve() << endl;