Deriving an iterator

[jono...@googlemail.com]

Hi there,

I've derived std::vector and std::vector::iterator
.

Unfortunately the derived iterator does not work as I would like.

///// test.cpp //////////////////////////////////////////////////

#include <iostream>
#include <vector>

using std::vector;

template<typename T>
struct Myvec : public vector<T> {
using vector<T>::vector;

class myiterator;

myiterator begin1() {return myiterator{static_cast<vector<T>>(*this).begin()}; }
myiterator end1() {return myiterator{static_cast<vector<T>>(*this).end()}; }
const myiterator begin1()const {return myiterator{static_cast<vector<T>>(*this).begin()}; }
const myiterator end1() const {return myiterator{static_cast<vector<T>>(*this).end()}; }

};

template<typename T>
struct Myvec<T>::myiterator : public vector<T>::iterator{
// CONSTRUCTOR
myiterator(const typename vector<T>::iterator& it) : vector<T>::iterator{it} {}
using vector<T>::iterator::iterator;
};


int main()
{
Myvec<int> vec = {1, 2, 3};
for (Myvec<int>::myiterator it = vec.begin1(); it != vec.end1(); ++it) {
std::cout << *it << '\n';
}
return 0;
}

///////////////////////////////////////////////////////


Compile (on linux) with
c++ -std=c++11 -o test test.cpp

EXPECTED ("wanted") OUTPUT:
1
2
3

The OUTPUT I unfortunately get:
0
0
3

What's wrong with the code?
How does one get it working so that the derived iterator is usable (in the way I want)

Thanks.

[jono...@googlemail.com]

OK, here's how to SOLVE it:

Instead of
static_cast<vector<T>>(*this).begin()
USE THIS:
static_cast<vector<T>*>(this)->begin()

But the more important question: why does only the 2nd one work correctly???

Thanks for any help and pointers ;) on this!!

J.




Below is the full code:
///////////////////////////////////////////////////////

#include <iostream>
#include <vector>

using std::vector;

template<typename T>
struct Myvec : public vector<T> {
using vector<T>::vector;

class myiterator;

myiterator begin1() {return myiterator{static_cast<vector<T>*>(this)->begin()}; }
myiterator end1() {return myiterator{static_cast<vector<T>*>(this)->end()}; }
const myiterator begin1()const {return myiterator{static_cast<vector<T>*>(this)->begin()}; }
const myiterator end1() const {return myiterator{static_cast<vector<T>*>(this)->end()}; }

[Victor Bazarov]

On 12/27/2014 1:17 PM, jono...@googlemail.com wrote:
> OK, here's how to SOLVE it:
>
> Instead of
> static_cast<vector<T>>(*this).begin()
> USE THIS:
> static_cast<vector<T>*>(this)->begin()
>
> But the more important question: why does only the 2nd one work correctly???
>
> Thanks for any help and pointers ;) on this!!
>
> J.

The first expression creates a temporary vector<T>, which disappears as
soon as the full expression has been evaluated. The second expression
does not create a temporary and instead yields an iterator for the
existing ('this') vector. The second one works correctly most likely
because you actually need the iterator for the existing vector and not
for some temporary (which becomes invalid as soon as the temporary is
destroyed, BTW).

V
--
I do not respond to top-posted replies, please don't ask

[Paavo Helde]

jono...@googlemail.com wrote in
news:392547a6-e783-4b69...@googlegroups.com:

> OK, here's how to SOLVE it:
>
> Instead of
> static_cast<vector<T>>(*this).begin()
> USE THIS:
> static_cast<vector<T>*>(this)->begin()
>
> But the more important question: why does only the 2nd one work
> correctly???

As Victor explained, the first one makes a copy. To avoid the copy, one
can also cast to a reference:

static_cast<vector<T>&>(*this).begin()

That said, std::vector (and other STL containers) are not really meant
for derivation. If you want to build on top of them, use them as data
members, not as bases. This will work out better in long term (short
rationale: public derivation leaks out the inner guts of std::vector,
which will overcomplicate your class' interface and make it hard to
maintain its class invariants).

Cheers
Paavo

[jono...@googlemail.com]

On Saturday, December 27, 2014 9:10:41 PM UTC+1, Victor Bazarov wrote:
> On 12/27/2014 1:17 PM, jono...@googlemail.com wrote:
> > OK, here's how to SOLVE it:
> >
> > Instead of
> > static_cast<vector<T>>(*this).begin()
> > USE THIS:
> > static_cast<vector<T>*>(this)->begin()
> >
> > But the more important question: why does only the 2nd one work correctly???

>

> The first expression creates a temporary vector<T>, which disappears as
> soon as the full expression has been evaluated. The second expression
> does not create a temporary and instead yields an iterator for the
> existing ('this') vector. The second one works correctly most likely
> because you actually need the iterator for the existing vector and not
> for some temporary (which becomes invalid as soon as the temporary is
> destroyed, BTW).
>

Oh yes, I think you're right! The first version....
static_cast<vector<T>>(*this) goes ahead and actually USES A COPY CONSTRUCTOR to create a temporary copy of the vector; and then calls begin() to return an iterator for this temporary. Definitely not what I wanted.

I need an iterator to the initial vector, therefore using a pointer and deref via -> is what is needed.

[jono...@googlemail.com]

On Saturday, December 27, 2014 10:19:29 PM UTC+1, Paavo Helde wrote:
> static_cast<vector<T>&>(*this).begin()

Ah yes, thank!

>
> That said, std::vector (and other STL containers) are not really meant
> for derivation. If you want to build on top of them, use them as data
> members, not as bases. This will work out better in long term (short
> rationale: public derivation leaks out the inner guts of std::vector,
> which will overcomplicate your class' interface and make it hard to
> maintain its class invariants).

This makes sense. Because as it happens, I'm working with a derived vector, that handles pointers and automatically deletes them in the destructor. And "SURPRISE": I have to also overload 'erase' to also do deletes! And other stuff as well... probably. Which is exactly what you say in the rationale.

Thanks.

[Jens Müller]

On 27.12.2014 22:28, jono...@googlemail.com wrote:
> Because as it happens, I'm working with a derived vector, that handles pointers and automatically deletes them in the destructor. And "SURPRISE": I have to also overload 'erase' to also do deletes! And other stuff as well... probably. Which is exactly what you say in the rationale.

Then a better solution is probably to use a normal std::vector and put
suitable smart pointers (e.g. std::unique_ptr) into it.

[jono...@googlemail.com]

Challange: try that suggestion of yours using std::unique_ptr
;)


Here's a solution (no peeking!):


///////////////////////////////////////

#include <vector>
#include <memory>
#include <initializer_list>

using std::vector;

template<typename T>

struct pvector : public vector<std::unique_ptr<T>>{
// using vector<std::unique_ptr<T>>::vector;
pvector(std::initializer_list<T*> lst);

void push_back(T *p) { static_cast<vector<std::unique_ptr<T>> *>(this)->push_back(std::unique_ptr<T>{p}); }
};

template<typename T>
pvector<T>::pvector(std::initializer_list<T*> lst) {
for (T* p: lst) {
push_back(p);
}
}

///////////////////////////////////////

Yip, what makes this interesting is the fact that std::unique_ptr cannot be copied (in order to prevent double frees!)

[Paavo Helde]

jono...@googlemail.com wrote in
news:bbfe390e-b2d5-4531...@googlegroups.com:

> On Saturday, December 27, 2014 11:27:32 PM UTC+1, Jens Müller wrote:
>> On 27.12.2014 22:28, jono...@googlemail.com wrote:
>> > Because as it happens, I'm working with a derived vector, that
>> > handles
> pointers and automatically deletes them in the destructor. And
> "SURPRISE": I have to also overload 'erase' to also do deletes! And
> other stuff as well... probably. Which is exactly what you say in the
> rationale.
>>
>> Then a better solution is probably to use a normal std::vector and
>> put suitable smart pointers (e.g. std::unique_ptr) into it.
>
> Challange: try that suggestion of yours using std::unique_ptr
> ;)
>
>
> Here's a solution (no peeking!):
>
>
> ///////////////////////////////////////
>
> #include <vector>
> #include <memory>
> #include <initializer_list>
>
>
> using std::vector;
>
> template<typename T>
> struct pvector : public vector<std::unique_ptr<T>>{

I think you did not get the idea, which was to just use std::vector of a
suitable element type, not to derive from it.

typedef vector<std::unique_ptr<mytype>> pvector;

If std::unique_ptr does not work for you by some reason, consider
std::shared_ptr.

Cheers
Paavo

[Nobody]

On Sat, 27 Dec 2014 15:19:12 -0600, Paavo Helde wrote:

> That said, std::vector (and other STL containers) are not really meant for
> derivation. If you want to build on top of them, use them as data members,
> not as bases. This will work out better in long term (short rationale:
> public derivation leaks out the inner guts of std::vector, which will
> overcomplicate your class' interface and make it hard to maintain its
> class invariants).

Another issue is that STL classes typically don't have virtual
destructors.

[jono...@googlemail.com]

Maby you did not realize this...

Please give me some examples using your type pvector above.
Do some push_back and initializer list...

Just to give some examples of what does NOT work:

typedef std::vector<std::unique_ptr<int>> pvector;

pvector vec1 = {new int{1}}; // error
pvector vec2 = {std::unique_ptr<int>(new int{2})}; // error
pvector vec3;
vec3.push_back(new int{3}); // error
vec3.push_back(std::unique_ptr<int>(new int{3})); // error

That's why (I think that) in this particular case you need the version given by me in a previous post.

As I mentioned above...

[Paavo Helde]

jono...@googlemail.com wrote in
news:849029b8-1fd7-4878...@googlegroups.com:

> Please give me some examples using your type pvector above.
> Do some push_back and initializer list...

The unique_ptr constructor is explicit, so one needs to spell it out when
creating these objects.

This works for me:

#include <vector>
#include <memory>

typedef std::unique_ptr<int> pint;
typedef std::vector<pint> pvector;

int main() {
pvector vec3;
vec3.push_back(pint(new int{ 3 }));
}


The initializer list version is failing though. I am not so familiar with
the initializer lists to say if and how they should work. Maybe my
compiler (VS2013) is still deficient.

>> Yip, what makes this interesting is the fact that std::unique_ptr
>> cannot
> be copied (in order to prevent double frees!)

Yes, but it can be moved, which makes it possible to put it in (C++11-
compatible) STL containers.

Cheers
Paavo

[Paavo Helde]

Nobody <nob...@nowhere.invalid> wrote in
news:pan.2014.12.28....@nowhere.invalid:

This is only a (potential) issue if you allocate them dynamically, but
this would not make much sense anyway. The STL containers perform any
needed dynamic allocations in inside, so there is no need to allocate the
container objects themselves dynamically. If I want to transfer them
somehow from one memory location to another there are swap() and C++11
move operations.

OK, maybe you have a point in that the people feeling an urge to derive
from std::vector might feel a similar urge to allocate it dynamically
(and delete unsafely) ;-)

Cheers
Paavo