General goal
I manage a collection of objects (Collection
of Real
as simple example). Then I defined iterators on my collection. That means : iterator
, const_iterator
, reverse_iterator
and const_reverse_iterator
. In this example, I will only pay attention on iterator
and const_iterator
, the two others are very similar.
After that, I would like to define a filter on my collection, which keeps or not the elements with respect to a specific condition. As exemple, keep only the Real
instances with a positive value. I also would like to iterate on my collection on the kept elements only.
How I implemented the collection
For this example, my object in the collection is very simple. The goal is just having an object instead of a native type :
struct Real
{
public:
double r;
};
Then I define my collection without having to know the real container inside :
class Collection
{
public:
typedef std::vector<Real>::iterator iterator;
typedef std::vector<Real>::const_iterator const_iterator;
private:
std::vector<Real> data;
public:
Collection() : data() {}
Collection(unsigned long int n) : data(n) {}
Collection(unsigned long int n, const Real& x) : data(n,x) {}
Collection::iterator begin() { return this->data.begin(); }
Collection::iterator end() { return this->data.end(); }
Collection::const_iterator begin() const { return this->data.begin(); }
Collection::const_iterator end() const { return this->data.end(); }
};
This is working very well in this simple example :
int main()
{
Collection c(5);
double k = 1.0;
for(Collection::iterator it = c.begin(); it != c.end(); ++it)
{
it->r = k;
k *= -2.0;
}
std::cout << "print c with Collection::iterator" << std::endl;
for(Collection::iterator it = c.begin(); it != c.end(); ++it)
std::cout << it->r << std::endl;
std::cout << "print c with Collection::const_iterator" << std::endl;
for(Collection::const_iterator it = c.begin(); it != c.end(); ++it)
std::cout << it->r << std::endl;
return 0;
}
And this program writes the expected output :
print with Collection::iterator
1
-2
4
-8
16
print with Collection::const_iterator
1
-2
4
-8
16
How I implemented the filter
Now I want to create an abstract filter, having a reference or pointer to a collection, having iterators, and having an abstract function accepting values through the filter. For this first step, I only wrote the class without the iterators :
class CollectionFilter
{
private:
Collection& col;
public:
CollectionFilter(Collection& c) : col(c) {}
virtual ~CollectionFilter() {}
Collection& collection() { return this->col; }
iterator begin() { /* todo */ }
iterator end() { /* todo */ }
const_iterator begin() const { /* todo */ }
const_iterator end() const { /* todo */ }
virtual bool accept(const Real& x) const = 0;
};
Then, it's quite easy to create a new filter implementing a specific condition :
class CollectionFilterPositive : public CollectionFilter
{
public:
CollectionFilterPositive(Collection& c) : CollectionFilter(c) {}
virtual ~CollectionFilterPositive() {}
virtual bool accept(const Real& x) const { return x.r >= 0.0; }
};
Before implementing the iterators in the filter, I have some remarks / questions.
- This filter works on a non-const
Collection&
, then, are thebegin() const
andend() const
function really required ? And if yes, why ? - I can't apply the filter on a
const Collection&
, but it's clearly required for my goal. What could be a good way to do that ? Have I to duplicate the classCollectionFilter
to a classCollectionFilterConst
with a very similar code ? Moreover this solution is quite confusing for the user having to inherit from two similar classes.
Then, let's go to the implementation of the iterators. For this example, I only wrote the iterator
and not the const_iterator
. I add this to my class :
class CollectionFilter
{
public:
class iterator
{
private:
CollectionFilter* filter;
Collection::iterator iter;
public:
iterator(CollectionFilter* f, Collection::iterator i) : filter(f), iter(i) {}
iterator(const iterator& i) : filter(i.filter), iter(i.iter) {}
iterator& operator = (const iterator& i) { this->filter = i.filter; this->iter = i.iter; return *this; }
iterator& operator ++ ()
{
if(this->iter != this->filter->collection().end())
{
do
{
++this->iter;
} while(this->iter != this->filter->collection().end() && !this->filter->accept(*this->iter));
}
}
iterator operator ++ (int) { /* similar */ }
Real& operator * () { return *this->iter; }
Collection::iterator operator -> () { return this->iter; }
bool operator == (const iterator& i) const { return this->iter == i.iter; }
bool operator != (const iterator& i) const { return this->iter != i.iter; }
};
public:
iterator begin()
{
Collection::iterator it = this->col.begin();
if(!this->accept(*it)) ++it;
return CollectionFilter::iterator(this,it);
}
iterator end()
{
Collection::iterator it = this->col.end();
return CollectionFilter::iterator(this,it);
}
};
This is also working well on this simple example
int main()
{
Collection c(5);
double k = 1.0;
for(Collection::iterator it = c.begin(); it != c.end(); ++it)
{
it->r = k;
k *= -2.0;
}
std::cout << "print c with CollectionFilterPositive::iterator" << std::endl;
CollectionFilterPositive fc(c);
for(CollectionFilterPositive::iterator it = fc.begin(); it != fc.end(); ++it)
std::cout << it->r << std::endl;
return 0;
}
giving the expected output :
print with CollectionFilterPositive::iterator
1
4
16
Again, some questions :
- Am I totally wrong with this approach ?
- I suppose I have to duplicate the code of
CollectionFilter::iterator
to implementCollectionFilter::const_iterator
with only small modifications. Is there a way to avoid duplication of this code (written 8 times, if I count the duplicated classCollectionFilterConst
and the reverse iterators) ? - I don't feel comfortable with the const-correctness of my code. Do you see some problems ?
Thanks in advance !
I would suggest to drop the
CollectionFilter
class, and instead have aCollection::filter_iterator_tmpl
template class, with two instantiationsCollection::filter_iterator
andCollection::const_filter_iterator
.Collection::filter_iterator_tmpl
could be implemented like this:Polymorphism can be added by letting
Predicate
be a polymorphic functionoid with avirtual bool PolymorphicPredicate::operator(Real) const
function.Collection
would then define the actual filter iterators:Boost implements a generic "Filter Iterator" in a similar way: http://www.boost.org/doc/libs/1_46_1/libs/iterator/doc/filter_iterator.html As a standalone class, not as part of a container class.
About const-correctness: Containers in C++ (
std::vector
,std::map
,std::string
, etc) own their content objects: They create and delete them, and need to make sure that with const access to the container, you also only get const access to the content objects. They need to be implemented so as to enforce this, because the underlying pointers by which they access their allocated storage don't have this notion of ownership. This is why they need to have two versions of iterators (iterator
andconst_iterator
). The iterators themselves don't own the object: With const access to aniterator
, you cannot advance the iterator, but you still get non-const access to the object.Questions 1/2:
CollectionFilter
is problematic because it does not own the objects that it provides access to, yet const/non-const access to the filter should only give const/non-const access to the object. Because it holds a reference to theCollection
, and it should work for both const and non-const access to theCollection
, two versionsCollectionFilter
andConstCollectionFilter
would then be needed with that approach.Question 4: Once there is a split from a const-correct container object to two classes for const and non-const access, there necessarily is some code duplication. Templates avoid having to implement both versions manually. There are also some additional complications, such as comparing an
iterator
to anconst_iterator
, and constructing aconst_iterator
from aniterator
but not the other way around...Questions 3/5: See above.