Issue with inheritance and virtual destructors in C++

Question

So I have two classes, one is abstract, and one is not.

The abstract class is Iterator, and the concrete one is LinkedListIterator. The code for both is at the bottom of the post.

The issue I'm having is with the code as it is below, I get 1 error in my LinkedListIterator at the last line of the destructor saying

undefined reference to `Iterator<int>::~Iterator()'

Now I tried commenting out the virtual ~Iterator() destructor, and no errors but I get a warning saying:

Class '[C@800c1' has virtual method 'remove' but non-virtual destructor

So my question is: Do I need a virtual destructor in the abstract Iterator base class? I've read that you should always have one, but in this case the destructor in LinkedListIterator merely sets values, it doesn't free anything...

Thanks!

Iterator Code:

template<class T>
class Iterator {

    public:
        //~Constructors/Destructors------------//
        /*
         * Destroys necessary resources.
         */
        virtual ~Iterator() = 0;

        //~Methods-------------------//
        /*
         * Informs the user whether there are more elements to be iterated
         * over in a List.
         *
         * @return true if there are more elements to iterate over, false otherwise.
         */
        virtual bool hasNext() = 0;

        /*
         * Gets the next element to iterate over.
         *
         * @return the next element in the iteration.
         */
        virtual T next() = 0;

        /*
         * Adds an element to the List being iterated over.
         *
         * @param element the element to add.
         * @return true if successful, false otherwise.
         */
        virtual bool add(T element) = 0;

        /*
         * Removes the element last returned by next from
         * the List being iterated over.
         *
         * @return true if successful, false otherwise.
         */
        virtual bool remove() = 0;
};

Relevant LinkedListIterator Code (it's a longish class):

template<class T>
class LinkedListIterator : public Iterator<T> {

    private:
        //~Data Fields---------------------------------//
        /*
         * Pointer to the node that the iterator is currently at.
         */
        Node<T>* current;
        /*
         * Pointer to the LinkedList being iterated through.
         */
        LinkedList<T>* list;
        /*
         * Boolean value indicating whether next has been called since
         * the last remove operation.
         */
        bool nextCalled;

    public:
        //~Constructors/Destructors------------------//
        /*
         * Constructor for LinkedListIterator, takes in a pointer to a Node
         * to initialize current to point to (intended to be the head of the
         * the LinkedList).
         *
         * @param theList pointer to the LinkedList being iterated through.
         */
        LinkedListIterator(LinkedList<T>* theList) {

            current = theList->head;
            list = theList;
            nextCalled = false;
        }

        /*
         * Destructor, resets pointer values to 0.
         */
        ~LinkedListIterator() {

            current = 0;
            list = 0;
        }

Answer 1

Others have answered how to solve the error you're seeing, but your question

Do I need a virtual destructor in the abstract Iterator base class?

has not been answered.

The answer is it depends on how you (and other users) intend to use these classes. There's nothing wrong with not having a virtual destructor and everything will work correctly until someone decides to delete an instance of LinkedListIterator through an Iterator pointer. For example:

Iterator<int> *iter = new LinkedListIterator<int>(...);
delete iter; // undefined behavior if Iterator destructor is not virtual

But since you're implementing an iterator class, the chances of someone dynamically allocating an instance, let alone attempting a polymorphic deletion on it, should be minimal. However, since you've already got other virtual functions, I can't think of a downside to declaring the destructor virtual either.

Answer 2

Your base class should have a virtual destructor, but not a pure virtual destructor (*).

Pure virtual functions (i.e. functions marked as virtual and with the = 0 suffix appended to their signature) have no implementation. However, the destructor of a base class always needs to be invoked by the destructor of a subclass, and you should provide a definition for it (possibly, an empty one):

template<class T>
class Iterator {

    public:
        //~Constructors/Destructors------------//
        /*
         * Destroys necessary resources.
         */
        virtual ~Iterator() { }
    ...

Also see this Q&A on StackOverflow for related information.

(*) As mentioned in the linked Q&A, it is possible to have a pure virtual destructor (for which a definition is still necessary), but I do not find it a particularly good programming practice.

Answer 3

You need to provide a definition for your virtual base destructor regardless of whether it is pure.

virtual ~Iterator() { }

should fix it.

PS: purely FYI there is a std link list, std::list

Answer 4

Despite the fact that your destructor is pure virtual, you do need to provide an implementation in the base class for the type to be usable.

Just add an empty destructor for Iterator; you don't need to lose the "pure" destructor (although it doesn't make much sense since you already have other pure virtual methods).