STL <vector> problems

[Clarus the DogCow]

I'm trying to create an application that uses a lot of arrays that need to be redefined, so I turned to <vector> in the STL library. The code I'm using to try it isnt working well (at all) and would like some suggestions how I _should_ be implementing vectors. Here it is:

#include <stdlib.h>
#include <stdio.h>
#include <iostream.h>
#include <vector>

#define MAIN

using namespace std;

struct sarray
{
int val;
vector<int> vec_one;

sarray();
~sarray();
};

sarray::sarray()
{
printf("Made\n");
}

sarray::~sarray()
{
printf("Killed\n");
}

void main(void)
{
vector<sarray> minime;
sarray *bob;
int x;

for (x=0;x<3;x++)
{
bob=new sarray;

bob->vec_one.clear();
bob->vec_one.push_back(9);
bob->val=x*2;
minime.push_back(*bob);
}
for (x=0;x<minime.size();x++)
{
printf("%d\n",minime[x].val);
printf("%d\n",minime[x].vec_one.size());
}
minime.clear();
delete bob;
}

[NCarter]

Not totally sure what kind of mechanism you're trying to create here, but I can perhaps offer some hints.

For starters, be aware that STL containers hold their contents by value, not by reference. That means that if you say 'vector<Class>', every time you put something into the vector (and especially if you try to insert into it) you're doing a copy, so you'll cause the copy constructor or the assignment operator for Class to be invoked. That's bad if Class has non-trival members such as pointers which need special copying. If you didn't supply your own copy constructor and assignment operator your Class objects will end up in a mess, and even if you did you'll have expensive copy operations happening all the time.

A simple alternative would be to say 'vector<Class*>' (because that way only the pointers would be copied), but that can cause problems if the objects being pointed to get deleted unexpectedly. The most correct way, apparently, is to use a reference counted pointer class - there's a good one supplied with CodeWarrior in a file called 'RefCountedPtrArray.h'. With this class you would say 'vector<_RefCountedPtr<Class> >' (notice the extra space between the two closing angle brackets), and you'll get efficient copying of only the pointer class without risking accidental deletion of the Class objects.

Another thing to consider is using list instead of vector. Unless you really need access by index (which doesn't seem to be the case in your example), list is probably more efficient.

If you're a bit vague about using the STL, it's worth reading a book on the subject. There are some nasty pitfalls that can really mess things up, and because it's entirely template based, it's very hard to debug (or even to understand the compiler error messages)!

[Clarus the DogCow]

I agree 200% with it being "very hard to debug (or even to understand the compiler error messages)!"

It makes it hard to understand as well.

What I really need is a simple array that can have items added to after an initial loading. Someone recomended STL to me, so I got a copy and tried it out.
I have been using pointers previously, but I was doing something wacky to get it compile:

-SNIP-
T *elements;
elements=(T*)malloc(sizeof(T)*size);
for (x=0;x<size;x++){
elements[x]=*(new T);
}
-SNIP-

A pointer to a temporary T? This isn't what I really want to do.
I needed to have the constructor class run, but wasn't sure how.

I'm fairly new to programming, and I think I might be going too far over my head too quickly.

[henryj]

STL vectors are pretty easy to use. You may have a case of thinking it's harder than it actually is and making things complicated for your self.

The code you posted is fine. It compiles and runs without a problem. What isn't working? What are you trying to achieve?

[ClarustheDogCow]

The code works, but I get a size of 0 for vec_one. This shouldn't be.
Try changing "printf("%d\n",minime[x].vec_one.size()); " to "printf("%d\n",minime[x].vec_one); "
That doesn't work (crashes when I run it), at least with MrCpp (which I know is far from perfect).

I have a medium sized amount (about 2000 lines of code) of arrays that I keep getting memory problems with the way of pointers I posted. I thought STL's vector would be a better alternative and more widley accepted (and known-its a sourceforge project).

What I'm trying to achieve is the correct way of creating and deleting struct heiarchy within a vector. I have several instances where I need a parent to have a vector as a child, and this is why I have the little vector<int>. Basically, I'm trying to figure out the best way of doing this without constantly changing all of my code that I have, trying to create a sample to apply to my other code.

Sorry about my rambling.

[henryj]

I get a size of 1...

henry% ./vec_test
Made
Made
Killed
Made
Killed
Killed
val 0
size 1
val 2
size 1
val 4
size 1
Killed
Killed
Killed
Killed


"printf("%d\n",minime[x].vec_one); "
You can't do this. printf'ing a vector is meaningless. I shouldn't compile and I find it hard to believe Mrcpp compiles it.

printf("%d\n",minime[x].vec_one[0]);

is probably what you want.


"I thought STL's vector would be a better alternative and more widley accepted (and known-its a sourceforge project)."
STL is part of the c++ standard. It has nothing to do with sourceforge unless you are using some ones port of it. Which one are you using?

Mrcpp is an excellent compiler as long as you don't need standards compliance. (It wont let you overload operator new[] for example) Because of this some things might not work as you expect although I don't remember having problems with the stl when I was using MPW.


Here is how I would have written your code. Actually I wouldn't have done it anything like this but this is a bit better.


#include <stdlib.h>
#include <stdio.h>
#include <vector>

#define MAIN

using namespace std;


struct sarray
{
sarray( int v)
: val(v)
{
vec_one.clear(); // unnecessary;
printf("Made\n");
}

~sarray()
{
printf("Killed\n");
}

// should be private
int val;
vector<int> vec_one;

};


void main(void)
{
vector<sarray> minime;
int x;

for (x=0;x<3;x++)
{
sarray bob( x * 2);

bob.vec_one.push_back(9);
minime.push_back(bob);
}

for (x=0;x<minime.size();x++)
{
printf("val %d\n",minime[x].val);
printf("size %d\n",minime[x].vec_one.size());
printf("%d\n",minime[x].vec_one[0]);
}

minime.clear();
}

Any questions?

[ClarustheDogCow]

I compiled it as a SIOW app (instead of a MPW tool) and it worked fine. Weird.

My code is going to be posted on sourceforge, so it would be easier for people to understand and recognize (as it is a standard).

But why are some being killed when they are being made?

[NCarter]

Originally posted by ClarustheDogCow
But why are some being killed when they are being made?

That's because you're using vector<sarray>, which is a vector of objects rather than a vector of (for example) pointers to objects. That means that any operations on the vector cause extra copies, constructions and destructions to occur (sometimes lots of them). As I said before, this is bad if your class isn't trivial, because the amount of time spent creating and destroying objects (and their subobjects) can be substantial.

Again, you don't really want a vector of pointers because it's too dangerous, but if you use a reference counted pointer it all becomes really easy and you only have one construction and one destruction per object. I found an example of such a class here:

http://www.kbcafe.com/articles/refcountptr.html

I'm suggesting this one because it looks a lot like the Metrowerks _RefCountedPtr class and I know that one works for me. I haven't had a chance to test it though. This page also tells you a bit about auto_ptrs, but I'll tell you now that you don't want to have vectors of auto_ptrs. They don't work with STL containers (I had this problem myself recently!).

With this class you would do something like:

vector<RefCountPtr<sarray> > minime;
minime.push_back(new sarray(...)); // sarray parameters omitted


Another advantage of this system is that when the vector is destroyed, any objects that are only referred to by the vector are also destroyed. However, if any other RefCountPtrs are pointing to the object it'll be retained as necessary and deleted only when all the other RefCountPtrs are deleted.

[Codemattic]

>>
Another thing to consider is using list instead of vector. Unless you really need access by index (which doesn't seem to be the case in your example), list is probably more efficient.
<<

Not always true - make sure to test it if speed is critical. Lists tend to blow cache coherence. Vectors - especially vectors of small objects or object pointers tend to keep cache coherence. That bit of wisdom was from one of the Game Programming Gems book.

hth,
Codemattic

[ClarustheDogCow]

Thanks. I think I'll find the right combination of list, vectors, and old fasioned arrays.
But it seems like if I wanted the best performance/efficiency ratio, I should make my own use of malloc-ed pointers. Is this what I should be doing? I.E.

struct sarray *tsad;
tsad=malloc(sizeof(struct sarray)*3);
tsad[x]=2;

[henryj]

I would not recommend you use arrays. Don't take offense but judging by the code you first posted you will have real problems with memory leaks which are much harder to debug than vectors.

If you are using arrays now just replace them with vectors. Vectors can be passed into functions that expect c style arrays. That's what they were designed for.

DON"T worry about performance until you can prove with profiling that it is a problem. No one can say that vectors are slow unless they can look at the profile of your code. People think that they know but they don't.

Using containers of pointers...

vector<sarray*>
instead of
vector<sarray>

can be problematic if you don't know what you are doing. Stick with the second until you can prove it's a problem.

I wouldn't bother with reference counting until you have more experience with the STL. I've been using the STL for years and never used ref counting.

Read an STL book. That is the only way you can make a correct decision about which container to use.

Give STL a go. It's a classic of library design and will make your code nicer, easy to use and it's fun in a twisted sort of way. And it will improve the rest of your c++.

Any questions?

[NCarter]

Originally posted by henryj
Using containers of pointers...

vector<sarray*>
instead of
vector<sarray>

can be problematic if you don't know what you are doing. Stick with the second until you can prove it's a problem.

I wouldn't bother with reference counting until you have more experience with the STL. I've been using the STL for years and never used ref counting.


That's fair enough. In most cases just using the container by value (vector<sarray>) will do the right thing, so it's by far the simpler solution, and that's definitely a good thing if you're just getting used to the STL. Just make sure that your constructor and destructor don't do anything funny (like opening files) because of the regularity with which they'll be called. Oh, and supply a copy constructor and assignment operator if necessary.

[Codemattic]

>>
struct sarray *tsad;
tsad=malloc(sizeof(struct sarray)*3);
tsad[x]=2;
<<

you might find it cleaner to use new and delete.


type * typePointer;

typePointer = new type [sizeOfArray];

// use array to do stuff
// i.e. typePointer[index]=something; etc...

//clean up
delete [] typePointer;

and yes that delete [] syntax looks strange but its correct.

hth,
Codemattic