Return to Revisions

You're reimplementing std::forward_list, so a good place to start would be the API documentation for forward_list. That'll show you a lot of deficiencies in your current approach:

• Naming. For example, getSize() should be size(), so that you can write generic algorithms that work with your class in addition to std::list, std::vector, etc (all of which have a size() method).

• Encapsulation. You provide a public member function getHead() that returns a raw pointer to an internal node. What do you expect the user to be able to do with a node<T>*?

• Composability. Your remove member function causes an exit(-1) on failure. It would be much much better to have remove throw an exception, and then allow the caller to catch and handle the exception — possibly with an exit, but possibly in some other way.

• Support common C++ idioms. Your list class doesn't provide begin() and end(), so it's not iterable; e.g. I can't say for (auto&& element : myList) { std::cout << element; }. If you provide iteration, then I can write this loop, so I won't need your printList member function anymore.

Finally, one more naming nit: As a member function, printList could just be named print. myList.print() is slightly better (shorter and less repetitive) than myList.printList(). And again it enables generic programming:

// You could overload this printTwice function for each container type:
void printTwice(const List& l)
{
 l.printList();
 l.printList();
}
void printTwice(const Vector& l)
{
 l.printVector();
 l.printVector();
}
// ^ But yuck. Prefer genericity:
template<class Container>
void printTwice(const Container& c)
{
 c.print();
 c.print();
}
// ^ In fact, prefer to use non-member functions for anything that
// doesn't require special access to the innards of the class:
template<class Container>
void printTwice(const Container& c)
{
 print(c);
 print(c);
}
// ^ In fact, prefer to spell the verb "print" in the standard way[*]:
template<class Container>
void printTwice(const Container& c)
{
 std::cout << c << std::endl;
 std::cout << c << std::endl;
}

[* – unless you're really concerned about efficiency, in which case you should probably stay away from iostreams; but at that point you're not doing generic programming anymore anyway]