Iterator class using a square list

I am trying to implement squarelist by using iteration that points to the head element. I've create this iterator class to make my iterator id directional. I just want to see if I implemented it correctly.

class iterator : public std::iterator
    {

    public:
        iterator(Node* p=nullptr) : _node(p){}
        ~iterator(){}

        Node* getNode(){return _node;}

        Object operator * () { return _node->data; }
        iterator & operator ++()
        {
            _node = _node->next;
            return *this; 
        }

        iterator operator ++(int)
        {
            iterator  retVal = *this;
            ++this;
            return retVal;
        }
        bool operator < (iterator const& rhs) const
        { 
            return _node < rhs._node; 
        }
        bool operator != (iterator const& rhs) const
        {
            return _node != rhs._node;
        }
        bool operator == (iterator const& rhs) const 
        {
            return _node == rhs._node;
        }
    };

A couple of real issues (rather than the imaginary ones previously pointed out):

Your code does not fully implement the requirements of iterator.

• You have not implemented the operator-> as required by the iterator concept.

• operator* should return by reference.

This is because *x = t is a requirement of iterator and without the reference this will not work as expected (or at all depending).

Without the container code its hard to tell if you can use the same iterator for both const and non-const iterator types. So sometimes its useful to provide const overloads of the above (but that will depend on usage).

see: http://www.sgi.com/tech/stl/trivial.html

You classify your iterator as

std::bidirectional_iterator_tag

This is an indication that you can go backwards and forwards with the iterator. In fact the concept means that your iterator must also support operator-- are well as operator++.

See: http://www.sgi.com/tech/stl/BidirectionalIterator.html

The link you provide indicates that a square list is circular. So moving forward all the time you may never reach the end. But without understanding (or seeing your implementation) it is hard to tell if the current implementation will work.

iterator & operator ++()
    {
        _node = _node->next;   // Will this ever reach a NULL
                               // Or do you have a special version of
                               // iterator that marks the end.
                               //
                               // Note: end is one past the last element
                               //       Which makes this doubly hard to
                               //       implement with a circular list.
                               //       But can be done if you use a sentinel.
                               //
                               // The default constructor suggests that the
                               // end is marked by a NULL but hard to be sure.
        return *this; 
    }

No point in defining destructor the default version will work perfectly. (As a side note: since it is not virtual there can be no derived types with alternative behaviors so making things symmetric is doubly useless).

~iterator(){}

This is perfectly fine.

iterator  retVal = *this;

But just as a personal preference I prefer:

iterator  retVal(*this);

As mentioned above (but without the container code it is hard to tell). Well this test work for the end iterator.

bool operator != (iterator const& rhs) const
    {
        return _node != rhs._node;
    }
    bool operator == (iterator const& rhs) const 
    {
        return _node == rhs._node;
    }

There is no requirement for bidirectional iterators to have a less than comparison. Unless you really want to store iterators in a sorted container I would leave this out.

Note: A random access iterator does have a requirement for a less than operator. But it has a precondition that it can only compare iterators that are reachable from each other.

see: http://www.sgi.com/tech/stl/RandomAccessIterator.html

bool operator < (iterator const& rhs) const
    { 
        return _node < rhs._node; 
    }

The second thing to consider is the sort ordering correct?

• You are defining the less than operator in term of pointer arithmetic. Unless the object are allocated by the same allocators from a known block this is actually undefined behavior. For pointer comparison to be valid they need to be in the same allocated block.

• Is this a good ordering? Youre ordering is based on their address (not the position in the container). So things may be in a jumbled up state in relation to everything else. This may be perfectly fine but something you should keep in mind.

WHY The following answers is a complete waste of time and WRONG

So please stop voting for it.

I learnt about its importance following discussions on one of the C++ working groups. It's a good practice to follow smarter people.

Yes but you have to also understand when to apply these principles. Some ideas contradict other ideas if you just use them bluntly. You need to understand each idea but also when to apply it.

The concept of operator symmetry is an important concept when used in the correct situation (no argument there). The problem is that iterators are not the correct situation and it plays no part.

But we have two competing concepts at play here. One: is symmetry is good. Two: auto conversion of types by the compiler is bad.

You have to balance the two concepts. In terms of iterators auto converting iterators is a very bad idea. You want to make the user of the iterator be explicit about any conversions (as the STL designers did (see below)). As a result the symmetry concept can not be used (as it requires auto conversion).
Why it makes no sense for auto conversion

```
Container x;
AltCont y;

// You can not compare iterators
// From different containers.
if (

StackExchange Code Review Q#41655, answer score: 12