Implementing an iterator (problem)

[mitrious]

I'm implementing a bi-directional iterator for a class (a simple list that holds strings) ... the book I'm using asked me to do so ... (I'll have to implement a version of the standard list class afterwards) ... but my iterators just won't work...

here's what I have

my iterator as defined inside my list class

class iterator : std::iterator<std::bidirectional_iterator_tag, std::string>{
	public:
		explicit iterator(std::string* p) : element(p){};
		iterator() : element(0) {};

		iterator& operator++(){++element; return *this;};
		iterator& operator--(){--element; return *this;};

		std::string& operator*(){return *element;};


		void operator=(std::string* pointer){element = pointer;};

		bool operator==(iterator& it){return *this == it;};
		bool operator!=(iterator& it){return *this != it;};
		bool operator==(std::string* it){return element == it;};
		bool operator!=(std::string* it){return element != it;};

	private:
		std::string* element;
	};

and one of the constructors

String_list::String_list(size_type n, std::string s){
	data = alloc.allocate(n);
	limit = data;
	size_t x = 0;
	while(x != n)
		++limit;
	uninitialized_fill(data, limit, s);
}

if I use pointers (that are random access iterators) I can make it work just fine ... so I can only assume the problem is inside my iterator class definition ...

I get no errors at compile time ... but when run the program ... nothing happens not even the "press any key to continue..." text appears ... can anyone tell me what I'm doing wrong?

[caut_baia]

Why don't you just make your own ?

#include <string>

template <typename T>
class Iterator {
   T* elem;
public:
   Iterator () : elem(NULL) {}
   explicit Iterator (const T& t) : elem(&t) {}
   ~Iterator () {}

   T& operator* ()  {
      return *elem;
    }
       //operators
 };

class String_List  {
   std::string st;
   friend class Iterator<String_List>;   
public:
   typedef Iterator<String_List> iterator;
   Iterator begin ();
   //etc
 };

[arkoenig]

Saying "it just won't work" doesn't really give readers much to go on.

However, I will say that your iterator has several problems:

1) It defines == and != as members rather than friends. Worse, it defined them only recursively. In other words, you're defining it1==it2 by saying that it's the value of it1==it2. That can't work.

2) You define prefix ++ and -- but not postfix ++ and --.

3) You define = with a pointer as its right-hand side but not an iterator.

4) None of your member functions are const, so you can't do anything useful with const iterators.

5) For some reason, you define == and != comparisons between interators and pointers, but not between pointers and iterators. I'm not sure why you want to bother, but it makes no sense to define only one of them.

I suggest fixing these problems before looking further.

[mitrious]

Why don't you just make your own ?

#include <string>

template <typename T>
class Iterator {
   T* elem;
public:
   Iterator () : elem(NULL) {}
   explicit Iterator (const T& t) : elem(&t) {}
   ~Iterator () {}

   T& operator* ()  {
      return *elem;
    }
       //operators
 };

class String_List  {
   std::string st;
   friend class Iterator<String_List>;   
public:
   typedef Iterator<String_List> iterator;
   Iterator begin ();
   //etc
 };

Sorry, but you lost me ... why should I define a Iterator<String_list> shouldn't it be a Iterator<std::string> with String_list being the container ?

and why wasn't my version working ? I don't understand why it wasn't working

[mitrious]

Saying "it just won't work" doesn't really give readers much to go on.

However, I will say that your iterator has several problems:

1) It defines == and != as members rather than friends. Worse, it defined them only recursively. In other words, you're defining it1==it2 by saying that it's the value of it1==it2. That can't work.

2) You define prefix ++ and -- but not postfix ++ and --.

3) You define = with a pointer as its right-hand side but not an iterator.

4) None of your member functions are const, so you can't do anything useful with const iterators.

5) For some reason, you define == and != comparisons between interators and pointers, but not between pointers and iterators. I'm not sure why you want to bother, but it makes no sense to define only one of them.

I suggest fixing these problems before looking further.

I'm trying to fix these things you pointed ... just one question ... about defining the == and != as friends ... how should I do that?

[arkoenig]

I'm trying to fix these things you pointed ... just one question ... about defining the == and != as friends ... how should I do that?

Go to your nearest C++ textbook and read about friend functions.

[mitrious]

Go to your nearest C++ textbook and read about friend functions.

That's everything I came up with so far:

template<class T> class Iterator{
        //all working
	template<class T>
	friend bool operator==(const Iterator<T>&, const Iterator<T>&);
	template<class T>
	friend bool operator!=(const Iterator<T>&, const Iterator<T>&);
	template<class T>
	friend bool operator==(const T*&, const Iterator<T>&);
	template<class T>
	friend bool operator!=(const T*&, const Iterator<T>&);
	template<class T>
	friend bool operator==(const Iterator<T>&, const T*&);
	template<class T>
	friend bool operator!=(const Iterator<T>&, const T*&);

	T* elem;
public:
	Iterator(): elem(NULL){}
	Iterator(T* pt): elem(pt){}
	Iterator(const T& t): elem(&t){}
	~Iterator(){}

	T operator*(){return *elem;}
	const T operator*()const {return *elem;}

	Iterator& operator++(){++elem; return *this;}
	Iterator& operator++(int){++elem; return *this;}
	Iterator& operator--(){--elem; return *this;}
	Iterator& operator--(int){--elem; return *this;}
	const Iterator& operator++() const{++elem; return *this;}
	const Iterator& operator++(int) const{++elem; return *this;}
	const Iterator& operator--() const{--elem; return *this;}
	const Iterator& operator--(int) const{--elem; return *this;}

	Iterator& operator=(const Iterator& it){elem = it.elem; return *this;}
	Iterator& operator=(const T*& pt){elem = pt; return *this;}
	Iterator& operator=(void*){elem = NULL; return *this;} /* used to to be able to initialize Iterators in the form "it = 0" for my list class.*/

	template<class P> /*used this for both conversions to void* to use Iterator as a condition and for conversions to char* */
	operator P(){
		return elem;
	}
};

But I got stuck when I got a message from unitialized_fill in this code

template <class T>
void List<T>::create(size_type n, const T& val){
	data = Alloc.allocate(n);
	avail = data;
	size_type i = 0;
	while(i != n){
		++avail;
		++i;
	}
	limit = avail;
	std::uninitialized_fill(data, avail, val);
}

The error I get is
"1>c:\Program Files (x86)\Microsoft Visual Studio 9.0\VC\include\memory(287) : error C2102: '&' requires l-value"

Any ideas on how to improve this Implementation?

[arkoenig]

Lines 27, 29, 31, 33: This technique of implementing operator++(int) and operator--(int) is wrong. These operators return the previous value of the object, which you can accomplish along these lines:

Iterator operator++(int) {  //Note: Iterator, not Iterator&
    Iterator ret = *this;  // Save a copy of *this
    ++elem;
    return ret;            // Return the saved copy
}

The error message you cite refers to line 287. Which line is that in what you posted?

[mitrious]

Lines 27, 29, 31, 33: This technique of implementing operator++(int) and operator--(int) is wrong. These operators return the previous value of the object, which you can accomplish along these lines:

Iterator operator++(int) {  //Note: Iterator, not Iterator&
    Iterator ret = *this;  // Save a copy of *this
    ++elem;
    return ret;            // Return the saved copy
}

The error message you cite refers to line 287. Which line is that in what you posted?

I fixed those lines thx ... (the operator++() and operator--() implementation is right tho?, isn't it?)

Well, it refers to the line 287 in the memory header ... it refers to the uninitialized_fill definition ... in my code the error is in the line whrere I use uninitialized_fill to initialize the elements in the container

[arkoenig]

And what are the types of data and avail?

Meanwhile... Are you saying that line 287 in the memory header is the call to uninitialized_fill?

[mitrious]

And what are the types of data and avail?

Meanwhile... Are you saying that line 287 in the memory header is the call to uninitialized_fill?

data and avail are of the type Iterator<T> where T will be the type that the container I"m implementing will hold

(yeah when I double click the error message ... it takes me to the memory header)

[arkoenig]

(yeah when I double click the error message ... it takes me to the memory header)

So we need to see the code in the memory header that it's complaining about.

[arkoenig]

Never mind, I think I found your problem. Lines 23 and 24:

T operator*(){return *elem;}
const T operator*()const {return *elem;}

have to return references.

[mitrious]

Never mind, I think I found your problem. Lines 23 and 24:

T operator*(){return *elem;}
const T operator*()const {return *elem;}

have to return references.

How?

[arkoenig]

T& operator*(){return *elem;}
const T& operator*()const {return *elem;}

[mitrious]

T& operator*(){return *elem;}
const T& operator*()const {return *elem;}

OH, thx ... now I'll work on some other problems I'm having with this container implementation, this was very helpful! thx

[arkoenig]

You're welcome.

Tracking down what part of the program is causing a compilation error during template instantiation can be a pain.

[mitrious]

You're welcome.

Tracking down what part of the program is causing a compilation error during template instantiation can be a pain.

It so can! but now I found out where my other problem s ... I'll try to find out why it's not working ... (I had to isolate all the parts in small useless programs to test what worked and what did not)