Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How do I make my iterator classes not look like container classes?

The premise

Say I have a container class Box which provides inner classes const_iterator and iterator. Because I want an iterator to be castable to a const_iterator, the latter inherits from the former:

class Box {
  // ...
public:
  class const_iterator : public std::iterator<std::random_access_iterator_tag, const int> { /* ... */ };
  class iterator : public const_iterator { /* ... */ };
  // ...
};

The problem

Now I want to test these classes using Google Test. Let's assert that the begin() and end() don't return the same thing:

const Box a;
EXPECT_NE(a.begin(), a.end());

Say hello to a compile error:

  • clang: no member named 'begin' in 'Box::const_iterator'
  • g++: ‘const class Box::const_iterator’ has no member named ‘begin’

The cause

Some research led me to this template in the Google Test source code (follow the link for expanded documentation):

typedef int IsContainer;
template <class C>
IsContainer IsContainerTest(int /* dummy */,
                            typename C::iterator* /* it */ = NULL,
                            typename C::const_iterator* /* const_it */ = NULL) {
  return 0;
}

The result of this template magic is that if arguments to EXPECT_* have iterator and const_iterator member classes, then the type is assumed to be a container class. Knowing this, Google Test can print pretty human-readable reports when expectations fail, which is nice.

However, there's this little detail:

// Note that we look for both C::iterator and C::const_iterator.  The
// reason is that C++ injects the name of a class as a member of the
// class itself (e.g. you can refer to class iterator as either
// 'iterator' or 'iterator::iterator').  If we look for C::iterator
// only, for example, we would mistakenly think that a class named
// iterator is an STL container.

so if I understand things right, this means that

  • Box::const_iterator has itself as a member class named const_iterator, and std::iterator as a memberclass named iterator.
  • Box::iterator has itself as a member class named iterator and Box::const_iterator as a member class named const_iterator.

Therefore both my iterator classes look like container classes to Google Test!

The question

How do I design my iterator classes to make them not look like containers?

Things I've tried:

  • Declaring the std::iterator superclass of const_iterator as private. This solves the problem for const_iterator by hiding the iterator member class, but it still doesn't let me pass a.begin() as a parameter to EXPECT_NE unless a is const. Seems like Google Test uses iterator begin() rather than const_iterator begin() const for some reason.
  • Removing the std::iterator superclass altogether. Is this a bad idea? I suppose I'll have to declare my std::iterator_traits manually, is there anything else I'll lose by not extending std::iterator?
  • Declaring the Box::const_iterator superclass of Box::iterator as private. This may or may not be an option since I'd have to redeclare methods I'd rather want to reuse (such as operator++).

Is there anything else I've overlooked?


The example

#include<iterator>
#include <memory> //unique_ptr<T>
#include <gtest/gtest.h>

class ThreeInts {
  std::unique_ptr<int[]> v;

  public:
  ThreeInts() : v(new int[3]) { v[0] = 0; v[1] = 1; v[2] = 2; };
  ThreeInts(int val) : ThreeInts() { v[0] = val; v[1] = val; v[2] = val; };

  bool operator==(const ThreeInts& other) const {
    return v[0] == other.v[0] && v[1] == other.v[1] && v[2] == other.v[2];
  }

  class const_iterator : public std::iterator<std::random_access_iterator_tag, const int> {
  protected:
    int* p;
  public:
    explicit const_iterator(int* p) : p(p) {}
    const_iterator& operator++() { ++p; return *this; }
    bool operator==(const const_iterator& rhs) const { return p == rhs.p; }
    bool operator!=(const const_iterator& rhs) const { return p != rhs.p; }
    int operator*() const { return *p; }
  };

  class iterator : public const_iterator {
  public:
    explicit iterator(int* p) : const_iterator(p) {}
    int& operator*() const { return *p; }
  };

  iterator begin() { return iterator(v.get()); }
  iterator end() { return iterator(v.get()+3); }
  const_iterator begin() const { return const_iterator(v.get()); }
  const_iterator end() const { return const_iterator(v.get()+3); }
};

TEST(ThreeInts, ThisTestCompilesAndPrettyFailureMessagesAreShown) {
  const ThreeInts a(1), b(2);
  ThreeInts c(1), d(2);
  EXPECT_EQ(a, b);
  EXPECT_EQ(a, c);
  EXPECT_EQ(c, d);
}

TEST(ThreeInts, ThisTestCompilesIfTheStdIteratorParentIsPrivate) {
  const ThreeInts a;
  EXPECT_NE(a.begin(), a.end());
}

TEST(ThreeInts, ThisTestAlsoCompilesIfTheStdIteratorParentIsPrivateButItIsAHassle) {
  ThreeInts a;
  ThreeInts::const_iterator beg = a.begin();
  ThreeInts::const_iterator end = a.end();
  //EXPECT_NE(beg, end); // Compile error unless the std::iterator superclass is private
}

TEST(ThreeInts, ThisTestDoesNotCompileEvenIfTheStdIteratorParentIsPrivate) {
  ThreeInts a;
  //EXPECT_NE(a.begin(), a.end());
}

int main(int argc, char **argv) {
  ::testing::InitGoogleTest(&argc, argv);
  return RUN_ALL_TESTS();
}
like image 329
Emil Lundberg Avatar asked Oct 13 '14 19:10

Emil Lundberg


1 Answers

ThreeInts::iterator should not inherit from ThreeInts::const_iterator, instead they should be implemented separately.

class ThreeInts::iterator : public std::iterator< std::random_access_iterator_tag, int> { ... }
class ThreeInts::const_iterator : public std::iterator< std::random_access_iterator_tag, const int> { ... }

The problem seems to be that otherwise ThreeInts::const_iterator both has members named const_iterator and iterator (aka the constructors). Also making the iterator inherit from const_iterator is not const-correct as the const_iterator should only hold a pointer/similar to const data. STL containers also keep the two iterators separate.

In that code, it would probably be sufficient to instead of defining iterator classes, simply define

using iterator = int*;
using const_iterator = const int*;
like image 122
tmlen Avatar answered Sep 17 '22 12:09

tmlen