Revisiting My Linked List

[Volatile Pulse]

Hey guys, it's been a while since I've even dabbled with C++ but I really wanted to get the motivation to be interested in it like I used to be. I thought a good start would be to revisit my old linked list I started making based off of the standard library container of the same name. I've been working fairly meticulously to get my uncompleted member functions filled in and completed.

I've been trying to make an all inclusive list of functions, basically rewriting all of the standard library functions. I recently found myself stuck on the unique function that uses a binary predicate. My code:

         // Declaration
         void unique(BinaryPredicate binary_pred);

         // ...snip...

// Definition
template<class T>
void vp::list<T>::unique(BinaryPredicate binary_pred) {
   for (iterator it = begin(); it != end(); it ++)
      if ((it != begin()) && (binary_pred(*it, it.current->prev->value)))
         erase(it --);
}

I believe my implementation is correct, however, I'm clearly missing BinaryPredicate. I quite honestly am not sure what I should be declaring, defining, or including and I'm hoping someone here can point me in the right direction. I also have the error logs as well:

||=== Build file: "no target" in "no project" (compiler: unknown) ===|
C:\Developer\Projects\list.cpp|101|error: 'BinaryPredicate' has not been declared|
C:\Developer\Projects\list.cpp|642|error: variable or field 'unique' declared void|
C:\Developer\Projects\list.cpp|642|error: 'BinaryPredicate' was not declared in this scope|
C:\Developer\Projects\list.cpp||In function 'int main()':|
C:\Developer\Projects\list.cpp|705|error: invalid conversion from 'bool (*)(int, int)' to 'int' [-fpermissive]|
C:\Developer\Projects\list.cpp|101|note:   initializing argument 1 of 'void vp::list<T>::unique(int) [with T = int]'|
C:\Developer\Projects\list.cpp|708|error: invalid type argument of unary '*' (have 'int')|
||=== Build failed: 5 error(s), 0 warning(s) (0 minute(s), 4 second(s)) ===|

I also have some other general questions about the code and would like someone to work with me one on one as well. I also have the entire code here: http://pastebin.com/jCgsky0Q

[coder777]

I would think that BinaryPredicate is either another template parameter or a typedef like this:

typedef bool (*BinaryPredicate)(int, int);

[Volatile Pulse]

I can't believe I didn't think of that sooner. I had other typedef's but being away from the code for so long I just didn't spend enough time reviewing the old code first.
Modified to:
typedef bool (*BinaryPredicate) (T, T);

Side note, I need to sit down and map the code out again. Thanks again coder.

[Volatile Pulse]

I'm staring here looking at iterator reverse_iterator const_reverse_iterator and can't figure out the best way to do this. Of course, I could define all three separately, but that seems like a waste of time and overkill. I also considered inheritance, however, it didn't appear to be the solution I was looking for. typedef was another option, but I definitely can't see how that would work in this situation.

So far, I have all three separately defined and completely independent of each other. Is there a better, cleaner way to do this? I feel there should be and I'm just not able to put 2 + 2 together. My understanding of the three is that they're only slightly different in some of their calls, but almost everything else is identical. For reference purposes, here is the iterator code:

/** Iterator definitions **/
template<class T>
vp::list<T>::iterator::iterator() :
   current(NULL) {}

template<class T>
vp::list<T>::iterator::iterator(node *n) :
   current(n) {}

template<class T>
typename vp::list<T>::reference vp::list<T>::iterator::operator ->() {
   return current->value;
}

template<class T>
typename vp::list<T>::reference vp::list<T>::iterator::operator *() {
   return current->value;
}

template<class T>
bool vp::list<T>::iterator::operator ==(const iterator it) const {
   return (current == it.current);
}

template<class T>
bool vp::list<T>::iterator::operator !=(const iterator it) const {
   return !operator == (it);
}

template<class T>
typename vp::list<T>::iterator& vp::list<T>::iterator::operator ++() {
   if (current)
      current = current->next;

   return *this;
}

template<class T>
typename vp::list<T>::iterator vp::list<T>::iterator::operator ++(int) {
   iterator result = *this;

   if (current)
      current = current->next;

   return result;
}

template<class T>
typename vp::list<T>::iterator& vp::list<T>::iterator::operator --() {
   if (current)
      current = current->prev;

   return *this;
}

template<class T>
typename vp::list<T>::iterator vp::list<T>::iterator::operator --(int) {
   iterator result = *this;

   if (current)
      current = current->prev;

   return result;
}

[coder777]

I'm staring here looking at iterator reverse_iterator const_reverse_iterator

You forgot the const_iterator.

Is there a better, cleaner way to do this?

This is a case for super inheritance:

template<class T>
struct base_iterator
{
  bool operator ==(const iterator it) const;
  bool operator !=(const iterator it) const;
...
};

template<class T>
struct const_iterator : base_iterator<T>
{
  typename const_reference operator ->() const; // Note: const
  typename const_reference operator *() const;

  void operator ++(); // Note: void works and simplifies things.
  void operator ++(int)
  {
    operator ++();
  }
...
};

template<class T>
struct iterator : const_iterator<T>
{
  typename reference operator ->();
  typename reference operator *();
...
};

// Repeat it for reverse_iterator 

Notice that I used void operator. It is not standard conform and means that you cannot cascade this operator. I recommend this.
If you want to be standard conform you need to reimplement the operator for the derived type.
This kind of inheritance has the advantage that you can switch from iterator to const_iterator any time (but not back).