Prefix and postfix ++ on custom iterator doing the same

Question

First of all I'm fairly new to C++ so if this is a beginner coding mistake then I'm sorry.

I'm currently working on a graphing class for a homework I got at school. I'm supposed to be able to store edges in a set, array and linked list. As I have done all of these in a separate classes I'm now trying to make them all work in one through templating. Everything works fine for ie. std::set, but when i use my own linked list implementation it somehow fails - it looks like my iterators get messed up somewhere, and both prefix and postfix operators on them result in the same behavior (in a for loop). I'll also add that I'm not using std::list because I'm supposed to make my own implementation of linked list.

My current implementation of the iterator:

template<typename T>
class Iterator{
    public: node<T>* pointer;
    public:
        Iterator(): pointer(0){}
        Iterator(node<T>* _pointer): pointer(_pointer){}

        Iterator<T> operator++()    { pointer = pointer->next; }
        Iterator<T> operator++(int) { pointer = pointer->next; }

        bool operator!=(Iterator<T> rval){ return !(pointer == rval.pointer); }
        bool operator==(Iterator<T> rval){ return (pointer == rval.pointer); }

        node<T>* operator()(){ return pointer; }

        T operator*(){ return pointer->data; }

};

Single linked list node:

template <typename T>
struct node{
    node(): next(0){}
    node(T val): data(val), next(0){}
    node(node<T>* _next): data(0), next(_next){}
    node(T val, node<T>* _next): data(val), next(_next){}

    T data;
    node<T>* next;
};

And how my list class implements begin() and end():

typedef Iterator<T> iterator;
iterator  begin()   { return iterator(new node<T>(b)); }
iterator  end()     { return iterator(); } 

Note that b points to the first element in the linked list

And finally how I'm accessing the elements (this is in a different class that includes the list):

void tree_recurse_f(int node, std::ofstream* file, int level = 0){
   [some output code here]
   typename T::iterator it;
   for (it = Database[node].first.begin(); it != Database[node].first.end(); ++it){
      tree_recurse_f(*it, file, (level+1));
   }
}

Database is an std::map<int,std::pair<>> and .first points to the type specified by T (set, list or vector)

Now to the problems:

1. Somehow with the current implementation of the lists' begin(), it points to an empty node in the output function (++it, and it++ result in the same thing)
2. Changing the begin() to return iterator(b) seems to remove the error in the for loop, though ++it and it++ both result in the same thing
3. I've managed to discover these two errors by testing only the list class - if i implement it into the graphing class it enters a never ending loop in the output function (*it always points to 0 and doesn't seem to increase with ++it)

Looks like some strange stuff with the iterator to me (especially the fact that alone it works, but inside the other class it doesn't)

// if someone was curious I'm loosely following the linked list tutorial on http://www.cplusplus.com/articles/Lw6AC542/

Answer 1

Your prefix and postfix operator do the same thing because you defined them to do the same thing:

Iterator<T> operator++()    { pointer = pointer->next; }
Iterator<T> operator++(int) { pointer = pointer->next; }

The code is identical, but most importantly, it has Undefined Behavior, because your functions are supposed to return a value of type Iterator<T> and, instead, they return nothing. Per Paragraph 6.6.3/2 of the C++11 Standard:

[...] Flowing off the end of a function is equivalent to a return with no value; this results in undefined behavior in a value-returning function.

You should change your prefix iterator into something like this:

Iterator<T> operator++() 
{ 
    pointer = pointer->next; 
    return *this;
}

And your postfix iterator into something like this:

Iterator<T> operator++(int) 
{ 
    node<T>* previous = pointer;
    pointer = pointer->next; 
    return Iterator<T>(previous);
}

Also, if I understand your design correctly, I really don't think you should be doing this:

iterator begin() { return iterator(new node<T>(b)); }

I would rather do:

iterator begin() { return iterator(b); }

Answer 2

The operator++ implementations here don't have a return statement. The return value is the key difference between pre- and post-increment. Pre-increment takes an iterator, increments it, and returns the new iterator value. It typically returns by reference, because the original iterator is the same as the returned value. Post-increment takes an iterator, squirrels it away, increments the original iterator, and returns the saved copy. This typically returns by value, because it returns an iterator that's different from the iterator it was applied to. So change the signatures to match these:

Iterator<T>& operator++();
Iterator<T> operator++(int);

and change the implementations to match what I described.

Answer 3

Your prefix and postfix ++ operators aren't actually returning anything. I'm surprised your compiler isn't complaining.

The prefix ++ operator should just return *this. The postfix ++ operator should create a temp copy of *this before the modification and then return that copy.