Re: simple vector question
brendan.morris@gmail.com wrote:
class BmTrack {
BmTrack();
BmTrack(BmDetection* detection, double deltaT, double sigma1,
double sigma2, int* track_total);
~BmTrack();
private:
int track_num; //track number
int track_class; //track classification
int track_class2;
CvKalman* kf; //kalman filter structure
std::vector<double> x; //track locations for each frame
std::vector<double> y;
double v; //average track velocity
double v2;
int lane_num; //lane number
int track_life; //count down to removal of track
CvMat* W; //class membership
CvMat* logW; //log for probabililty
CvMat* Meas; //measurement history
int flag_image; //tells if track image saved
IplImage* image; //image of object
CvRect bb; //bounding box
};
typedef std::vector<BmTrack*> BmTrack_vector;
I have a few problems here: you are passing around pointers in the
constructor, but what does that mean? Also, you store pointers as members,
what does that mean? There are typically two things that a pointer can
mean:
1. ownership
2. reference
When ownership is meant, you definitely need a copy constructor and
assignment operator, the compiler-generated ones are not sufficient. At
the very least, you should declare them as private and not implement them,
so they don't get used accidentally.
The size of the vector is constantly changing by removing elements from
within the vector.
You know that a vector is (mostly) a simple array allocated with new[],
right? That means, that when you remove or insert in the middle,
surrounding elements have to be moved, making this inefficient. I would
normally use a std::list in such cases and, if possible, not use pointers
then. Since you always remove a lot of elements in a loop (see below) you
could instead copy all elements you want to keep(!) to a new vector and
afterwards swap those two.
The way I'm dong it looks something like this.
BmTrack* t;
for(i=num_tracks-1; i>=0; i-=1) {
if(dead_track[i]) {
t = tracks.at(i);
delete t;
tracks.erase(tracks.begin()+i);
}
}
What is 'dead_track'? I guess it is a vector of booleans that marks tracks
that can be removed. In that case, you have a logical error here: as soon
as you remove an element from 'tracks', the index 'i' in 'dead_tracks'
doesn't resemble the index 'i' in 'tracks'!
BmTrack_vector tmp;
tmp.reserve(tracks.size());
for(...)
if(you_want_to_keep(tracks[i]))
tmp.push_back(tracks[i])
else
delete tracks[i];
tracks.swap(tmp);
Am I doing something wrong here. I don't have a copy or assignment
method defined for BmTrack.
You are not copying them, as you are using pointers, so this doesn't
matter. It is however good practice to have copying and assignment under
control, i.e. either implement them or disable them, unless the
compiler-generated ones are okay.
And is it a bad idea to create the BmTrack* t variable outside of
my loop?
Yes, the general rule is to keep the scope of variables as small as
possible. BTW: this also applies to loop-variables, which are typically
declared in the loop header:
for( int i=0; i!=limit; ++i)
...
You did that in one loop, but not in the other. In case you need to target
a buggy compiler that doesn't get the scope of such vars right, you should
still be able to work around it like this:
/** Inject another scope for compilers that don't properly scope
variables declared in a for loop. */
#if defined(_MSC_VER) && _MSC_VER<1300
# define for if(false){}else for
#endif
Further, why do you use vector::at() instead of vector::operator[]()? The
former does additional bounds-checking on the passed index, but if that is
necessary, it means your algorithm is faulty!
Uli
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]