0

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;
        }
Ethan
  • 1,206
  • 3
  • 21
  • 39

4 Answers4

6

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.

Community
  • 1
  • 1
Andy Prowl
  • 124,023
  • 23
  • 387
  • 451
  • This is probably all he needs, but the destructor *could be* pure virtual, though he'd still have to provide a definition. – Praetorian Feb 19 '13 at 19:45
  • @Praetorian: True, that's also explained in the linked Q&A. However, I don't find it a very good programming practice to have a pure virtual destructor with a definition. That's why I preferred not to mention it, but yes, you're right. – Andy Prowl Feb 19 '13 at 19:47
  • 1
    I agree, especially since he has a bunch of other pure virtual functions; can't think of a reason why the destructor would have to be pure virtual as well. – Praetorian Feb 19 '13 at 19:52
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

111111
  • 15,686
  • 6
  • 47
  • 62
3

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).

Jon
  • 428,835
  • 81
  • 738
  • 806
2

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.

Praetorian
  • 106,671
  • 19
  • 240
  • 328
  • 1
    @Cheers :) Almost missed it myself the first time since I (like the others, I assume) stopped reading the question after seeing the error message. – Praetorian Feb 19 '13 at 20:07
  • damn I had already marked another answer, but I read this and had to switch....Because I have a function in LinkedList (not listed above) called getIterator() that returns a dynamically allocated instance of a Iterator in EXACTLY the way you described it!! SO THIS DEFINITELY APPLIES TO ME!!!! – Ethan Feb 19 '13 at 20:21