I'm trying to convert an iterator class I have to be stl compatible so that it can be used with the stl algorithms. In the following simple (and frankly useless) example, which should print the values 0 to 5 inclusive, I am getting the following errors,
ISO C++ forbids incrementing a pointer of type ‘Iterator (*)()‘
and,
invalid conversion from ‘Iterator (*)()’ to ‘int‘
What am I doing wrong?
Thanks.
#include <iterator>
#include <algorithm>
#include <iostream>
class Iterator : public std::iterator<std::bidirectional_iterator_tag, int> {
public:
Iterator(int i = 0) : val(i) {
if(val<0 || val>5) throw;
}
bool operator==(Iterator const& rhs) const {
return (val==rhs.val);
}
bool operator!=(Iterator const& rhs) const {
return !(*this==rhs);
}
Iterator& operator++() {
if(val!=6)
++val;
return *this;
}
Iterator operator++(int) {
Iterator tmp (*this);
++(*this);
return tmp;
}
Iterator& operator--() {
if(val!=-1)
--val;
return *this;
}
Iterator operator--(int) {
Iterator tmp (*this);
--(*this);
return tmp;
}
int operator* () const {
if(val==-1 || val==6) throw;
return val;
}
private:
int val;
};
Iterator begin() {
return Iterator();
}
Iterator end() {
return ++Iterator(5);
}
void print(int i) {
std::cout << i << std::endl;
}
int main(int argc, char* argv[]) {
std::for_each(begin,end,print);
}
You are passing the functions begin and end to std::for_each, instead of the iterators that these functions would return:
std::for_each(begin,end,print);
It should be:
std::for_each(begin(),end(),print);
Also note that the empty throw statements, like in if(val==-1 || val==6) throw;, will not do anything good. You have to throw something, like throw std::out_of_range("out of bounds").
First, you should be passing the iterators that are returned by begin() and end() not the functions themselves:
int main(int argc, char* argv[])
{
std::for_each(begin(),end(),print);
}
Secondly, it would be useful to have a templated Iterator class:
template<class T>
class Iterator : public std::iterator<std::bidirectional_iterator_tag, int>
{
public:
typedef T value_type; //notice this here :D
Iterator(value_type t = 0) : val(t)
{
if(val<0 || val>5) throw; //never hardcode something like that :S
}
bool operator==(Iterator const& rhs) const
{
return (val==rhs.val);
}
bool operator!=(Iterator const& rhs) const
{
return !(*this==rhs);
}
Iterator& operator++()
{
if(val!=6) //never hardcode something like that :S
++val;
return *this;
}
Iterator operator++(value_type)
{
Iterator tmp (*this);
++(*this);
return tmp;
}
Iterator& operator--()
{
if(val!=-1) //never hardcode something like that :S
--val;
return *this;
}
Iterator operator--(value_type)
{
Iterator tmp (*this);
--(*this);
return tmp;
}
value_type operator* () const
{
if(val==-1 || val==6) throw; //never hardcode something like that :S
return val;
}
private:
value_type val;
};
Third, you might not really want to have an iterator class on its own like that. Here is an example of what you can do (note an iterator class is a bit lower) :
#include <algorithm>
template<class T>
class List
{
public:
typedef T value_type;
typedef std::size_t size_type;
private:
struct Knot
{
value_type val_;
Knot * next_;
Knot(const value_type &val)
:val_(val), next_(0)
{}
};
Knot * head_;
size_type nelems_;
public:
//Default constructor
List() throw()
:head_(0), nelems_(0)
{}
bool empty() const throw()
{ return size() == 0; }
size_type size() const throw()
{ return nelems_; }
private:
Knot * last() throw() //could be done better
{
if(empty()) return 0;
Knot *p = head_;
while (p->next_)
p = p->next_;
return p;
}
public:
void push_back(const value_type & val)
{
Knot *p = last();
if(!p)
head_ = new Knot(val);
else
p->next_ = new Knot(val);
++nelems_;
}
void clear() throw()
{
while(head_)
{
Knot *p = head_->next_;
delete head_;
head_ = p;
}
nelems_ = 0;
}
//Destructor:
~List() throw()
{ clear(); }
//Iterators:
class iterator
{
Knot * cur_;
public:
iterator(Knot *p) throw()
:cur_(p)
{}
bool operator==(const iterator & iter)const throw()
{ return cur_ == iter.cur_; }
bool operator!=(const iterator & iter)const throw()
{ return !(*this == iter); }
iterator & operator++()
{
cur_ = cur_->next_;
return *this;
}
iterator operator++(int)
{
iterator temp(*this);
operator++();
return temp;
}
value_type & operator*()throw()
{ return cur_->val_; }
value_type operator*() const
{ return cur_->val_; }
value_type operator->()
{ return cur_->val_; }
const value_type operator->() const
{ return cur_->val_; }
};
iterator begin() throw()
{ return iterator(head_); }
iterator begin() const throw()
{ return iterator(head_); }
iterator end() throw()
{ return iterator(0); }
iterator end() const throw()
{ return iterator(0); }
//Copy constructor:
List(const List & lst)
:head_(0), nelems_(0)
{
for(iterator i = lst.begin(); i != lst.end(); ++i)
push_back(*i);
}
void swap(List & lst) throw()
{
std::swap(head_, lst.head_);
std::swap(nelems_, lst.nelems_);
}
List & operator=(const List & lst)
{
List(lst).swap(*this);
return *this;
}
//Conversion constructor
template<class U>
List(const List<U> &lst)
:head_(0), nelems_(0)
{
for(typename List<U>::iterator iter = lst.begin(); iter != lst.end(); ++iter)
push_back(*iter);
}
template<class U>
List & operator=(const List<U> &lst)
{
List(lst).swap(*this);
return *this;
}
//Sequence constructor:
template<class Iter>
List(Iter first, Iter last)
:head_(0), nelems_(0)
{
for(;first!=last; ++first)
push_back(*first);
}
};
#include <iostream>
using std::cout;
using std::endl;
int main()
{
const char MAX_LIMIT = 127;
List<short> listA;
//...
List<char> listB = listA; //without the conversion constructor this would not go very far!
for(char i = 0; i < MAX_LIMIT; ++i)
listB.push_back(i);
for(List<char>::iterator iter = listB.begin(); iter != lstB.end(); ++iter)
cout << *iter << endl;
}
