problems with STL vector class

[sw]

Hi,

Is it possible to insert a class <vec> which has a vector<double>
member, into the vector<vec> veclist for e.g?

I've been getting compilation errors when trying to insert using the
vector method push_back() ->

c:\program files\microsoft visual studio\vc98\include\xutility(39) :
error C2679: binary '=' : no operator defined which takes a right-hand
operand of type 'const class vec' (or there is no acceptable
conversion)

c:\program files\microsoft visual studio\vc98\include\vector(170) : see
reference to function template instantiation 'void __cdecl
std::fill(class vec *,class vec *,const class vec &)' being compiled

c:\program files\microsoft visual studio\vc98\include\xmemory(34) :
error C2558: class 'vec' : no copy constructor available

c:\program files\microsoft visual studio\vc98\include\xmemory(66) : see
reference to function template instantiation 'void __cdecl
std::_Construct(class vec *,const class vec &)' being compiled
Error executing cl.exe.
I've already had an overloaded operator= for my vec class and i dont
really understand where the error lies in.

Here is my source code:

class <vec> header file

public:

vec();
vec(vec&);
vec(int, int); //dimension, vecid
vec(int, int, vector<double>); //dimension, vecid, coords
vec(int, int, vector<double>, vector<int>); //dimension, vecid,
coords, list of conected vecs
~vec();

void setvecid(int);
void setveccoord(vector<double>);
void setvecconnect(vector<int>);
void setvec(int, int, vector<double>, vector<int>);
void setvec(vec&);
int getdim();
int getvecid();
vector<double> getcoord(); //get the point coordinates
vector<int> getvecconnect(); //get the list of connected vecs
void addconnectvec(int); //add connected vec to the vecconnect list
void delconnectvec(int); //remove connected vec from list
bool searchvec(int); //search for vec in the vecconnect list
void sortvecconnect(); //sort the list in the

vec operator=(vec&);
In my main(), i declare a global variable:

vector<vec> globalvecs;

I get the compile error when i use the statement:

globalvecs.push_back(tmpvec);

inside the following block of code:

for(i = 0; i < numVec; i++)
{
vec tmpvec(dimension, i); //temp vec used to add to the global veclist
vector<double> tmpcoord; //vector to hold temp coordinates b4 setting

//tmpvec's coord

for(j = 0; j < dimension; j++)
{
read >> x; //where x is of type double
tmpcoord.push_back(x);
}
tmpvec.setveccoord(tmpcoord);

globalvecs.push_back(tmpvec);

}

Advance thanks for your replies!
Regards,

sw

[Rade]

The signature for a copy constructor is:

vec(vec const &);

and not

vec(vec &);

Rade

[Abecedarian]

sw wrote:

vec operator=(vec&);

vec& operator=(const vec&);

::A::

[Rolf Magnus]

sw wrote:

Hi,

Is it possible to insert a class <vec> which has a vector<double>
member, into the vector<vec> veclist for e.g?
Sure.
I've been getting compilation errors when trying to insert using the
vector method push_back() ->

c:\program files\microsoft visual studio\vc98\include\xutility(39) :
error C2679: binary '=' : no operator defined which takes a right-hand
operand of type 'const class vec' (or there is no acceptable
conversion)

c:\program files\microsoft visual studio\vc98\include\vector(170) : see
reference to function template instantiation 'void __cdecl
std::fill(class vec *,class vec *,const class vec &)' being compiled

c:\program files\microsoft visual studio\vc98\include\xmemory(34) :
error C2558: class 'vec' : no copy constructor available

c:\program files\microsoft visual studio\vc98\include\xmemory(66) : see
reference to function template instantiation 'void __cdecl
std::_Construct(class vec *,const class vec &)' being compiled
Error executing cl.exe.
I've already had an overloaded operator= for my vec class and i dont
really understand where the error lies in.
You need to be const-correct.

Here is my source code:

class <vec> header file

public:

vec();
vec(vec&);
With this signature, you say that the copy constructor will change the
original. It shouldn't (and probably doesn't) do that, so make it const:

vec (const vec&);
Â*vec(int, int, vector<double>); //dimension, vecid, coords
Â*vec(int, int, vector<double>, vector<int>); //dimension, vecid, coords, list of conected vecs
You should consider changing those two into taking references to the
vectors. That way you'll save an additional copy of them including
their whole content. Same for the other functions that take vectors.
vec operator=(vec&);

Same as above. The compiler says it quite explicitly:
"no operator defined which takes a right-hand operand of type 'const
class vec'"

Your operator= signature suggests that it will modify the right-hand
side of the assignment (i.e. it is not const), and the vector wants
one that doesn't do that.

Another thing: Don't make operator= return by value. This means that it
will create another copy of the whole vec (using the copy constructor)
for generating the return value. Make it:

vec& operator=(const vec&);

or

const vec& operator=(const vec&);

[sw]

thanks for the replies! your explanation has made it much clearer -
havent touched c++ code in a while, gotten quite rusty :)

[sw]

I'm facing another problem with the statement

globalvecs.push_back(tmpvec);

I've made the changes suggested accordingly. Now, the push_back()
method of the vector doesnt seem to copy "<vec> tmpvec" properly into
globalvecs. The dimension and vecid are copied accordingly but the
vector<double> veccoord variable doesnt seem to be copied properly into
the globalvecs.

I've tested the tmpvec values before this statement and the coord
values do exist and are not null values.

My source codes are:

for(i = 0; i < numVec; i++)
{
vec tmpvec(dimension, i); //temp vec used to add to the global
veclist
vector<double> tmpcoord; //vector to hold temp coordinates b4
setting tmpvec's coord
for(j = 0; j < dimension; j++)
{
read >> x;
tmpcoord.push_back(x);
}

tmpvec.setveccoord(tmpcoord);
cout<<"tmpvec coord's:"<< (tmpvec.getcoord()).at(0) << " "
<< (tmpvec.getcoord()).at(1) << " "
<< (tmpvec.getcoord()).at(2) << " "
<< (tmpvec.getcoord()).at(3) << " " <<endl;
globalvecs.push_back(tmpvec);

vector<double> tmpcoord1 = (globalvecs.at(i)).getcoord();

cout<< "globalvecs[" << i << "]" << endl
<< "dimension:" << globalvecs[i].getdim() << endl
<< "vecid:" << globalvecs[i].getvecid() << endl
<< "coords: "
<< tmpcoord1[0] << ", "
<< tmpcoord1[1] << ", "
<< tmpcoord1[2] << ", "
<< tmpcoord1[3] <<endl;
}
The output i get is:

tmpvec coord's:1 1.5 0.2 0
globalvecs[0]
dimension:4
vecid:0
coords: -1.45682e+144, -1.45682e+144, -1.41454e+144, 1.2798e-307
tmpvec coord's:5 1.3 0.1 4.00171
globalvecs[1]
dimension:4
vecid:1
coords: -1.45682e+144, -1.45682e+144, -1.41454e+144, 1.2834e-307
tmpvec coord's:2.5 1.4 3 2.37949
globalvecs[2]
dimension:4
vecid:2
coords: -1.45682e+144, -1.45682e+144, -1.41454e+144, 1.2798e-307
tmpvec coord's:2.3 5 1.5 5
globalvecs[3]
dimension:4
vecid:3
coords: -1.45682e+144, -1.45682e+144, -1.41454e+144, 1.2798e-307
tmpvec coord's:2.4 0 1.5 0.806838
globalvecs[4]
dimension:4
vecid:4
coords: -1.45682e+144, -1.45682e+144, -1.41454e+144, 1.2798e-307
tmpvec coord's:2.5 2.5 1.5 1.95897
globalvecs[5]
dimension:4
vecid:5
coords: -1.45682e+144, -1.45682e+144, -1.41454e+144, 1.2798e-307
Press any key to continue

[Rolf Magnus]

Rade wrote:

The signature for a copy constructor is:

vec(vec const &);

and not

vec(vec &);

Both are valid signatures for a copy constructor, but they still mean
different things.

[Rolf Magnus]

sw wrote:

I'm facing another problem with the statement

globalvecs.push_back(tmpvec);

I've made the changes suggested accordingly. Now, the push_back()
method of the vector doesnt seem to copy "<vec> tmpvec" properly into
globalvecs. The dimension and vecid are copied accordingly but the
vector<double> veccoord variable doesnt seem to be copied properly into
the globalvecs.

I've tested the tmpvec values before this statement and the coord
values do exist and are not null values.

How are your operator= and copy constructor of your vec implemented? Do you
actually copy the vector within them?

[sw]

My operator= is implemented as:

const vec& vec::operator=(const vec &right)
{
dim = right.dim;
vecid = right.vecid;
setveccoord(right.coord);
setvecconnect(right.vecconnect);
return *this;
}

with the setveccoord and setvecconnect as:

void vec::setveccoord(vector<double> point)
{
coord.clear();

int size = point.size();
int i;
double x;
for(i = 0; i < size; i++)
{
x = point[i];
coord.push_back(x);
}
}

void vec::setvecconnect(vector<int> vecs)
{
vecconnect.clear();
int size = vecs.size();
int i;
int x;
for(i = 0; i < size; i++)
{
x = vecs[i];
vecconnect.push_back(x);
}
}

The operator= seems to work, i tried the assigment between 2 <vec>
variables and the coord values are copied accordingly.

[Rade]

> Both are valid signatures for a copy constructor, but they still mean

different things.

Right, but in > 99% cases of const-correct programs you need the signature
with 'const'. I think (half-seriously) that the other form of copy
constructor should be outlawed for all but C++ experts.

Seriously: I am just afraid that your response may make the OP believe that
the two signatures of the copy constructor are equal, while in fact they are
not (that is true even on the language level, i.e. the compiler will
generate only the 'const' form when no copy constructor (of any form) is
present).

Rade

[sw]

I omitted the copy constructor accidentally. Here it is now:

vec::vec(const vec &newvec)
{
dim = newvec.dim;
vecid = newvec.vecid;
setveccoord(coord);
setvecconnect(vecconnect);
}

[Rolf Magnus]

sw wrote:

I omitted the copy constructor accidentally. Here it is now:

vec::vec(const vec &newvec)
{
dim = newvec.dim;
vecid = newvec.vecid;
setveccoord(coord);
setvecconnect(vecconnect);
}

Here we go. You probably meant:

setveccoord(newvec.coord);
setvecconnect(newvec.vecconnect);

Generally, you should prefer initialization over assignment in constructors.

vec::vec(const vec &newvec)
: dim(newvec.dim),
vecid(newvec.vecid),
coord(newvec.coord),
vecconnect(newvec.vecconnect)
{
}

Btw: Is that really all that your copy constructor is doing? If yes, you
should just remove it alltogether. If you don't define your own, the
compiler automatically generates one that does a memberwise copy, which is
what yours also does.