A question about vectors and pointers

[Mark Probert]

Hi ..

A basic template and pointer question for you.

I have a set of classes that look like (simplified):

class Foo {
public:
Foo *next;
Foo *other;
int n;

friend class FooList;
};

class FooList {
vector<Foo> foos;
void add(); // add a foo to the list
};

void FooList::add() {
Foo f = new Foo();
f->n = foos.size();

Foo *fp = foos.end(); // get the last added foo to link the pointers
if (fp != (Foo*)NULL)
fp->next = &f;
foos.push_back(f);
}

My problem is that the 'Foo *fp = foos.end();' is illegal.
How go I get the last item as it is stored?

Regards,

 

[CrayzeeWulf]

class Foo {
public:
Foo *next;
Foo *other;
int n;

friend class FooList;
};

All members of class Foo are public. Why do you need to make FooList a
friend ?

class FooList {
vector<Foo> foos;
void add(); // add a foo to the list
};

All members of class FooList are private. Is this really what you want ?

void FooList::add() {
Foo f = new Foo();
f->n = foos.size();

Foo *fp = foos.end(); // get the last added foo to link the pointers
if (fp != (Foo*)NULL)
fp->next = &f;
foos.push_back(f);
}

What are you trying to do here (please describe in words) ? If you really
want an iterator to the last member of foos, you can use "--foos.end()". Of
course, you have to make sure that foos.size() is non-zero.

Thanks,

 

[Val]

| If you really
| want an iterator to the last member of foos, you can use "--foos.end()". Of
| course, you have to make sure that foos.size() is non-zero.

Checking for non-zero is required indeed but not good enough. In general, I wouldn't use "--obj.end()" at all!
What if "vector<Foo>::iterator" is a Foo*? C++ won't allow you to modify temporary objects of built in types.

Check this out:
Foo* return_a_Foo() { //the code to do it }
myPointer = --return_a_Foo(); //Error?

However, if you are sure that "vector<Foo>::iterator" is a random access iterator then rewrite it like this:
"foos.end() -1 "
According to "Exceptional C++::Herb Sutter" you won't lose performance either.

 

[Mark Probert]

Check this out:
Foo* return_a_Foo() { //the code to do it }
myPointer = --return_a_Foo(); //Error?

However, if you are sure that "vector<Foo>::iterator" is a random access iterator then rewrite it like this:
"foos.end() -1 "
According to "Exceptional C++::Herb Sutter" you won't lose performance either.

Thanks, Val.

 

[Mark Probert]

All members of class Foo are public. Why do you need to make FooList a
friend ?

My apologies. The classes are a very simplified version of the real classes.
The members are private and there are other members and member functions.

What are you trying to do here (please describe in words) ? If you really
want an iterator to the last member of foos, you can use "--foos.end()". Of
course, you have to make sure that foos.size() is non-zero.

Thanks for that.

Regards,

 

[Stephen Howe]

void FooList::add() {

Foo f = new Foo();
f->n = foos.size();

Foo *fp = foos.end(); // get the last added foo to link the pointers

Undefined here.
You are assuming that an iterator can be converted to a pointer to Foo (or
is a pointer) and on some implementions that is not true. foos.end() returns
_just_ an iterator.

if (fp != (Foo*)NULL)
fp->next = &f;

And suppose that vector iterators are pointers in this implementation.
This is more undefined behaviour in that foos.end() is never supposed to be
dereferenced, yet you are setting fp->next, a big no-no.

foos.push_back(f);
}

My problem is that the 'Foo *fp = foos.end();' is illegal.
How go I get the last item as it is stored?

Foo *fp = &foos.back();

will do it. back() returns a reference to the last item stored in the vector
(assuming it is not empty). end() is always invalid and should never be
dereferenced for any container. begin() is always valid if the container is
not empty.

Stephen Howe

 

[Jeff Flinn]

Hi ..

A basic template and pointer question for you.

I have a set of classes that look like (simplified):

class Foo {
public:
Foo *next;
Foo *other;
int n;

friend class FooList;
};

class FooList {
vector<Foo> foos;

Your later usage implies that you meant:

void add(); // add a foo to the list
};

void FooList::add() {
Foo f = new Foo();
f->n = foos.size();

Foo *fp = foos.end(); // get the last added foo to link the pointers

You get a reference to the last item using

Foo* fp = foos.back();

if (fp != (Foo*)NULL)
fp->next = &f;
foos.push_back(f);
}

My problem is that the 'Foo *fp = foos.end();' is illegal.
How go I get the last item as it is stored?

While my intuition says that there may be better approaches to accomplishing
your final goal, the following will do what you were explicitly asking for.

class Foo
{
Foo* next;
Foo* other;
int n;
public:

Foo():next(0),other(0),n(0){}

Foo( std::vector<Foo*>& foos );

void AttachNext( Foo* aNext ){ next = aNext; }

...
};

class FooList
{
std::vector<Foo*> foos;

public:
void add(){ foos.push_back( new Foo( foos ) ); }
};

Foo::Foo( std::vector<Foo*>& foos ):next(0),other(0),n(foos.size())
{
if( !foos.empty() ) foos.back().AttachNext( this );
}

Jeff Flinn

 

[Andrew Koenig]

My problem is that the 'Foo *fp = foos.end();' is illegal.
How go I get the last item as it is stored?

That's not your only problem if you think it's meaningful to store the
address of a Foo that's an element of a vector.

Each time you execute push_back on the vector, the call might cause the
vector to reallocate. If it does, then every element of the vector will be
moved to another location in memory, and all pointers to elements of the
vector will become meaningless.

So if what you're trying to do is have a vector of objects that refer to
each other somehow, I can see three plausible choices:

1) Store vector indices rather than pointers.
2) Preallocate enough memory for the vector (using the "reserve" member
function) that it never reallocates.
3) Use a different container such as deque, which doesn't move its
elements around in memory.

 

[Mark Probert]

That's not your only problem if you think it's meaningful to store the
address of a Foo that's an element of a vector.

Many thanks, Andrew. I was not aware of that.
Deque it is ...