Iterators are pseudo-pointer types. That means they themselves are regular.
struct Iterator {
LinkedList *current;
LinkedList &c;
};
Here you mix references and pointers. This is a serious anti-pattern, as what does assignment do? There is no sensible answer.
I would remove the c
member entirely.
Next you need to broadcast an iterator type. Yours looks like a forward iterator. All end iterators can be equal.
Iterator begin(LinkedList *c) { return Iterator {c, *c}; }
Iterator end(LinkedList *c) { return Iterator {nullptr, *c}; }
These look ok. Just remove *c
.
Note that the name does not have to be Iterator
. begin
/end
must be defined in the namespace of LinkedList
, but the return type does not have to be.
Iterator &operator++(Iterator &p) { p.current = p.current->next; return p; }
I usually implement this as a member function, and implement both pre and post increment; post is implemented using pre and copy.
LinkedList *operator*(Iterator p) { return p.current; }
This is wrong. It should return *p.current
as a double&
.
bool operator!=(Iterator lhs, Iterator rhs) { return (lhs.current != rhs.current); }
sure. Also implement ==
as !(lhs!=rhs)
.
Look up the forward iterator concept and forward iterator tag. Include the types needed for std::iterator_traits
.
For other things to iterate, give the iterator a different name. This can be via a different namespace.
If the thing that differs is just the type of the value, you can make it a template pretty easy. Then you only have to manually write begin
/end
.
If the name of v
also changes, you could use ADL on a GetValue(List*)
function you write as a customization point.
Now, being usable in a ranged based for is different than being an iterator. Ranged based for is a tad easier; but the above upgrades you to a full forward iterator, which in turn reduces surprise when you try to use a std algorithm or basically anything else.
How I would write it:
// Iteration::LinkedListIterator<X> assumes that X is a linked list node
// with members ->next and ->value. If it isn't, override the customization
// points GetNextNode and GetListElement in the namespace of X.
namespace Iteration {
template<class List>
List* GetNextNode( List* l ) {
if (!l) return l;
return l->next;
}
template<class List>
decltype(auto) GetListElement( List* l ) {
return l->value;
}
template<class List>
struct LinkedListIterator {
using self=LinkedListIterator;
List *current;
self& operator++(){ current = GetNextNode(current); return *this; }
self operator++(int)&{ auto copy = *this; ++*this; return copy; }
decltype(auto) operator*() {
return GetListElement(current);
}
decltype(auto) operator*() const {
return GetListElement(current);
}
auto operator->() {
return std::addressof(GetListElement(current));
}
auto operator->() const {
return std::addressof(GetListElement(current));
}
friend bool operator==(self const& lhs, self const& rhs) {
return lhs.current == rhs.current;
}
friend bool operator!=(self const& lhs, self const& rhs) {
return lhs.current != rhs.current;
}
using iterator_category = std::forward_iterator_tag;
using value_type = std::decay_t<decltype(GetListElement(std::declval<List*>()))>;
using difference_type = std::ptrdiff_t;
using pointer = value_type*;
using reference = value_type&;
};
};
struct LinkedList {
double v;
LinkedList *next;
};
// customization point; the name of
double& GetListElement( LinkedList* l ) { return l->v; }
double const& GetListElement( LinkedList const* l ) { return l->v; }
Iteration::LinkedListIterator<LinkedList> begin( LinkedList* l ) {
return {l};
}
Iteration::LinkedListIterator<LinkedList> end( LinkedList* l ) {
return {nullptr};
}