CODEWITHSUNDEEP
c++iterator
fokenrute
fokenrute

Reputation: 739

Issue when implementing a map-like iterator in C++

I just finish to implement a generic skip list in C++ an I want to bring it an STL map-like iterator. The problem arise when overloading the "->" operator: it is required to return a reference to an iterator::value_type, which in my case is std::pair, with K being the key type and V the value type of my map-like container. The code is the following:

value_type& operator->() { return value_type(inner->key, inner->value); }

The compiler return me some hatred messages about references to temporary objects, and I completely agree with him; me question is: How the hell should I do to return a reference to a pair without having to put a pair member in my iterator class ?

Upvotes: 2

Views: 1367

Answers (4)

tyree731
tyree731

Reputation: 483

operator->() should only return values which are pointers or have overloaded operator->() functions defined for them. The point of this function is not so much to perform the '->' operation itself, but to provide the object or pointer for which the '->' operator will operate on.

In your case, to accomplish this you would need to return a pointer to pair. If you really would like to avoid having the pair stored inside your class, you could have the function return a smart pointer to a freshly allocated pair, that way no memory leaks occur during the operation.

Upvotes: 1

aschepler
aschepler

Reputation: 72271

For one thing, operator-> should return a pointer, or else something that has its own operator->() method. But if you don't have a value_type, that is, a pair<const K,V> stored, you can't easily return a pointer to it.

Can you change the definition of inner so that it is or contains a pair? You can add accessors like key() and value() in to avoid the confusing first and second in most places.

Or, depending on how much like std::map you need this to act, maybe you should use a different value_type typedef that describes what you actually do have.

Upvotes: 4

Jonathan Grynspan
Jonathan Grynspan

Reputation: 43472

value_type& operator->() { return value_type(inner->key, inner->value); }

Your return type is a reference to a value_type, but you return a locally-created instance, which would be destroyed at the end of the function/operator scope (i.e. the closing }.) So you return a garbage reference, which is why your compiler hates you.

If you instead return by value, you will have correct and functional code. (Whether or not you should return by value depends on the type of value_type.)

value_type operator->() { return value_type(inner->key, inner->value); }

Even better would be to maintain a single value_type as a member field of your class, but you say you don't want to, so this is the only other realistic option.

Upvotes: 1

cadolphs
cadolphs

Reputation: 9617

What you're doing in your snipped is creating a new instance of value_type. This object will of course be deleted once the function/method operator->() is left, which leaves your reference that you just returned pointing to nowhere.

What you should do is return a reference to the appropriate element already IN your list.

Upvotes: -1

Related Questions