views:

122

answers:

5
template <class Enum>
class EnumIterator {
 public:

  const Enum* operator-> () const {
    return &(Enum::OfInt(i));  // warning: taking address of temporary
  }

  const Enum operator* () const {
    return Enum::OfInt(i);     // There is no problem with this one!
  }

 private:
  int i;
};

I get this warning above. Currently I'm using this hack:

template <class Enum>
class EnumIterator {
 public:
  const Enum* operator-> () {
    tmp = Enum::OfInt(i);
    return &tmp;
  }
 private:
  int i;
  Enum tmp;
};

But this is ugly because iterator serves as a missing container.

What is the proper way to iterate over range of values?

Update: The iterator is specialized to a particular set objects which support named static constructor OfInt (code snippet updated).

Please do not nit-pick about the code I pasted, but just ask for clarification. I tried to extract a simple piece.

If you want to know T will be strong enum type (essentially an int packed into a class). There will be typedef EnumIterator < EnumX > Iterator; inside class EnumX.

Update 2: consts added to indicate that members of strong enum class that will be accessed through -> do not change the returned temporary enum.

Updated the code with operator* which gives no problem.

A: 

An iterator iterates on a specific container. The implementation depends on what kind of container it is. The pointer you return should point to a member of that container. You don't need to copy it, but you do need to keep track of what container you're iterating on, and where you're at (e.g. index for a vector) presumably initialized in the iterator's constructor. Or just use the STL.

paul
Good point. I updated the question, in my case there is no container.
Łukasz Lew
"An iterator iterates on a specific container." No, it iterates over a sequence. That doesn't have to be a container. See stream iterators for an example.
sbi
+1  A: 

There are many kind of iterators.

On a vector for example, iterators are usually plain pointers:

template <class T>
class Iterator
{
public:
  T* operator->() { return m_pointer; }

private:
  T* m_pointer;
};

But this works because a vector is just an array, in fact.

On a doubly-linked list, it would be different, the list would be composed of nodes.

template <class T>
struct Node
{
  Node* m_prev;
  Node* m_next;
  T m_value;
};

template <class T>
class Iterator
{
public:
  T* operator->() { return m_node->m_value; }

private:
  Node<T>* m_node;
};

Usually, you want you iterator to be as light as possible, because they are passed around by value, so a pointer into the underlying container makes sense.

You might want to add extra debugging capabilities:

  • possibility to invalidate the iterator
  • range checking possibility
  • container checking (ie, checking when comparing 2 iterators that they refer to the same container to begin with)

But those are niceties, and to begin with, this is a bit more complicated.

Note also Boost.Iterator which helps with the boiler-plate code.

EDIT: (update 1 and 2 grouped)

In your case, it's fine if your iterator is just an int, you don't need more. In fact for you strong enum you don't even need an iterator, you just need operator++ and operator-- :)

The point of having a reference to the container is usually to implement those ++ and -- operators. But from your element, just having an int (assuming it's large enough), and a way to get to the previous and next values is sufficient.

It would be easier though, if you had a static vector then you could simply reuse a vector iterator.

Matthieu M.
A: 

What does OfInt return? It appears to be returning the wrong type in this case. It should be returning a T* instead it seems to be returning a T by value which you are then taking the address of. This may produce incorrect behavior since it will loose any update made through ->.

stonemetal
Another good point. I updated the question - no side effects in methods.
Łukasz Lew
+1  A: 
Enum* operator-> () {
  tmp = Enum::OfInt(i);
  return &tmp;
}

The problem with this isn't that it's ugly, but that its not safe. What happens, for example in code like the following:

void f(EnumIterator it)
{
   g(*it, *it);
}

Now g() ends up with two pointers, both of which point to the same internal temporary that was supposed to be an implementation detail of your iterator. If g() writes through one pointer, the other value changes, too. Ouch.

Your problem is, that this function is supposed to return a pointer, but you have no object to point to. No matter what, you will have to fix this.

I see two possibilities:

  1. Since this thing seems to wrap an enum, and enumeration types have no members, that operator-> is useless anyway (it won't be instantiated unless called, and it cannot be called as this would result in a compile-time error) and can safely be omitted.
  2. Store an object of the right type (something like Enum::enum_type) inside the iterator, and cast it to/from int only if you want to perform integer-like operations (e.g., increment) on it.
sbi
I want to return temporary object!
Łukasz Lew
You can return a temporary object, but you should not return a pointer to a temporary object.
sbi
Exactly. But why can do: (*it).EnumMethod()but can't:it->EnumMethod()?
Łukasz Lew
That depends on your `operator*`. If it, too, is returning a reference to a temporary, it certainly has the very same problem as your `operator->` - regardless whether the compiler flags it with a warning or not.
sbi
Ah, I see you return an _object_ there, not a reference. So this problem doesn't occur with that operator.
sbi
Yes, I've heard that const reference temporaries have longer life in some situations, but I can't find it now. I guess that this does not apply to pointers. Unfortunately operator-> *requires* a pointer.
Łukasz Lew
No, it's that temporary objects, bound to const references, will have their lifetime extended until the reference dies, too. But this has nothing to do with your case here.
sbi
A: 

As there is no container I settled on merging iterator into my strong Enum. I init raw int to -1 to support empty enums (limit == 0) and be able to use regular for loop with TryInc.

Here is the code:

template <uint limit>
class Enum {
 public:
  static const uint kLimit = limit;

  Enum () : raw (-1) {
  }

  bool TryInc () {
    if (raw+1 < kLimit) {
      raw += 1;
      return true;
    }
    return false;
  }

  uint GetRaw() const {
    return raw;
  }

  void SetRaw (uint raw) {
    this->raw = raw;
  }

  static Enum OfRaw (uint raw) {
    return Enum (raw);
  }

  bool operator == (const Enum& other) const {
    return this->raw == other.raw;
  }

  bool operator != (const Enum& other) const {
    return this->raw != other.raw;
  }

 protected:
  explicit Enum (uint raw) : raw (raw) {
  }

 private:
  uint raw;
};

The usage:

class Color : public Enum <10> {
 public:
  static const Color red;

  // constructors should be automatically forwarded ...
  Color () : Enum<10> () {
  }

 private:

  Color (uint raw) : Enum<10> (raw) {
  }
};

const Color Color::red = Color(0);


int main() {
  Color red = Color::red;

  for (Color c; c.TryInc();) {
    std::cout << c.GetRaw() << std::endl;
  }
}
Łukasz Lew