Operator overloading and copy constructor. Can't find the error.

[clicwar]

A simple program with operator overloading and copy constructor:
#include <iostream>
#include <string>
using namespace std;

class Vector {
private:
float x,y;
public:
Vector(float u, float v);
Vector(void);
Vector operator+ (Vector a);
Vector(Vector &source);
void Show(void);
};
void Vector::Show(void) {
cout <<"(" << x <<"," <<y <<")";
}
Vector::Vector(float u, float v) {
x=u; y=v;
}
Vector::Vector(void) {
x=0; y=0;
}
Vector::Vector(Vector &source) {
x = (source.x)*2 ; y = (source.y)*2 ;
}
Vector Vector:perator+ (Vector a) {
Vector temp;
temp.x = x + a.x;
temp.y = y + a.y;
return temp;
}

int main(void) {
Vector a(3,1), b(5,2), c, d;
c = a+b;
d = a.operator+ (b);
cout << "Data members of the vector c: ";
c.Show();
Vector e(a+b);
cout <<endl << "Data members of the vector e: ";
e.Show();

return 0;
}


The compiler (g++ -pedantic -W -Wall) says:
teste.cpp: In function `int main()':
teste.cpp:36: error: no matching function for call to
`Vector::Vector(Vector)'
teste.cpp:24: note: candidates are: Vector::Vector(Vector&)
teste.cpp:37: error: no matching function for call to
`Vector::Vector(Vector)'
teste.cpp:24: note: candidates are: Vector::Vector(Vector&)
teste.cpp:40: error: no matching function for call to
`Vector::Vector(Vector)'
teste.cpp:24: note: candidates are: Vector::Vector(Vector&)
teste.cpp:21: note: Vector::Vector()
teste.cpp:18: note: Vector::Vector(float, float)


Without the copy constructor Vector::Vector(Vector &source) , it works
fine.

Would anyone know what is wrong in the code?
Thanks in advance.

 

[Jim Langston]

A simple program with operator overloading and copy constructor:
#include <iostream>
#include <string>
using namespace std;

class Vector {
private:
float x,y;
public:
Vector(float u, float v);
Vector(void);
Vector operator+ (Vector a);
Vector(Vector &source);
void Show(void);
};
void Vector::Show(void) {
cout <<"(" << x <<"," <<y <<")";
}
Vector::Vector(float u, float v) {
x=u; y=v;
}
Vector::Vector(void) {
x=0; y=0;
}
Vector::Vector(Vector &source) {
x = (source.x)*2 ; y = (source.y)*2 ;
}
Vector Vector:perator+ (Vector a) {
Vector temp;
temp.x = x + a.x;
temp.y = y + a.y;
return temp;
}

int main(void) {
Vector a(3,1), b(5,2), c, d;
c = a+b;
d = a.operator+ (b);
cout << "Data members of the vector c: ";
c.Show();
Vector e(a+b);
cout <<endl << "Data members of the vector e: ";
e.Show();

return 0;
}


The compiler (g++ -pedantic -W -Wall) says:
teste.cpp: In function `int main()':
teste.cpp:36: error: no matching function for call to
`Vector::Vector(Vector)'
teste.cpp:24: note: candidates are: Vector::Vector(Vector&)
teste.cpp:37: error: no matching function for call to
`Vector::Vector(Vector)'
teste.cpp:24: note: candidates are: Vector::Vector(Vector&)
teste.cpp:40: error: no matching function for call to
`Vector::Vector(Vector)'
teste.cpp:24: note: candidates are: Vector::Vector(Vector&)
teste.cpp:21: note: Vector::Vector()
teste.cpp:18: note: Vector::Vector(float, float)


Without the copy constructor Vector::Vector(Vector &source) , it works
fine.

Would anyone know what is wrong in the code?
Thanks in advance.

Change it to:
Vector::Vector( const Vector& a )
and it should work.

 

[clicwar]

First of all, thank you Jim. It worked ( but i don't know why the
simple addition of the const modifier can do that. Could you tell
me? ).

The output now is:
Data members of the vector c: (13,5)
Data members of the vector e: (13,5)

But it should be (16,6) for both c and e.
Any suggestions?
Thanks.

 

[Jim Langston]

First of all, thank you Jim. It worked ( but i don't know why the
simple addition of the const modifier can do that. Could you tell
me? ).

The output now is:
Data members of the vector c: (13,5)
Data members of the vector e: (13,5)

But it should be (16,6) for both c and e.
Any suggestions?
Thanks.

Well, I get even a weirder result with your code (26.3 or something). Not
sure why, but if you fix your operator+ it'll show the correct answer of
16.6

Vector operator+ (const Vector& a) const;

Operators and constructors have specific signatures. const is part of them
and I believe references are also. I really don't know what was going on
that was giving us the wrong results (I.E. no clue what the compiler was
donig) and don't really care, cause fixing the operator+ fixes the problem.

As to why it didn't work without your constructor having const, that's
because it didn't recognize it as a copy constructor.

Which makes me think, changing it from a copy to a reference probably means
your copy constructor is not working right, let me look at it...
Your copy constructor is mulitplying the values by 2? That's not what a copy
constructor is supposed to do.

Incidently, you can merge your 2 constructors into one.

Vector(float u, float v);

Vector::Vector(float u = 0, float v = 0): x(u), y(v)
{
}

Also, this is C++. We don't put (void) when there are no parameters to a
method/function. We just use () I.E.
void Show();

 

[I V]

A simple program with operator overloading and copy constructor: [...]
Vector::Vector(Vector &source) {
x = (source.x)*2 ; y = (source.y)*2 ;
}

Here, your "copy constructor" doesn't make an actual copy. Are you sure you
want to do that? It seems to me to be likely to cause confusion. Indeed, I
think this is why you're not getting the result you expect:

Vector a(3,1), b(5,2), c, d;
c = a+b;

Now, this calls a.operator+(b), which invokes the copy constructor on b;
so a.operator+ gets passed (10, 4); it then adds this to (3, 1), giving
(13, 5), and returns this, which itself invokes the copy constructor,
giving (26, 10). However, I think this last use of the copy constructor,
in the return, is optional; so the results of calling operator+ depend on
whether or not the compiler decides to optimize out the copy constructor.
This doesn't seem like a very good state of affairs.

 

[clicwar]

Jim, did you mean something like this?

#include <iostream>
#include <string>
using namespace std;

class Vector {
private:
float x,y;
public:
Vector(float u, float v);
Vector();
Vector operator+ (const Vector &a) const;
Vector(const Vector &source);
void Show();
};
void Vector::Show() {
cout <<"(" << x <<"," <<y <<")";
}
Vector::Vector(float u, float v) {
x=u; y=v;
}
Vector::Vector() {
x=0; y=0;
}
Vector::Vector(const Vector &source) {
x = (source.x)*2 ; y = (source.y)*2 ;
}
Vector Vector:perator+ (const Vector &a) const {
Vector temp;
temp.x = x + a.x;
temp.y = y + a.y;
return (temp);
}

int main() {
Vector a(3,1), b(5,2), c, d;
c = (a+b);
d = (a.operator+ (b));
cout << "Data members of the vector c: ";
c.Show();
Vector e((a+b));
cout <<endl << "Data members of the vector e: ";
e.Show();

return 0;
}

Did you use g++ ? Because now i'm getting (8,3) for both c and e.

Thanks for the patience and for the (void) tip.

Here, your "copy constructor" doesn't make an actual copy. Are you sure you
want to do that?

IV, how can i improve this? I came from a little C background and i
just learned how to do operator overloading.

A simple program with operator overloading and copy constructor: [...]
Vector::Vector(Vector &source) {
x = (source.x)*2 ; y = (source.y)*2 ;
}

Here, your "copy constructor" doesn't make an actual copy. Are you sure you
want to do that? It seems to me to be likely to cause confusion. Indeed, I
think this is why you're not getting the result you expect:

Vector a(3,1), b(5,2), c, d;
c = a+b;

Now, this calls a.operator+(b), which invokes the copy constructor on b;
so a.operator+ gets passed (10, 4); it then adds this to (3, 1), giving
(13, 5), and returns this, which itself invokes the copy constructor,
giving (26, 10). However, I think this last use of the copy constructor,
in the return, is optional; so the results of calling operator+ depend on
whether or not the compiler decides to optimize out the copy constructor.
This doesn't seem like a very good state of affairs.

 

[BobR]

Jim, did you mean something like this?

#include <iostream>
#include <string>
using namespace std;

class Vector { private:
float x,y;
public:
Vector(float u, float v);
Vector();
Vector operator+ (const Vector &a) const;
Vector(const Vector &source);
void Show();
};

void Vector::Show() {
cout <<"(" << x <<"," <<y <<")";
}
Vector::Vector(float u, float v) {
x=u; y=v;
}

Doesn't look like you know about 'initialization lists'. Try this:

Vector::Vector( float u, float v ) : x(u), y(v) {} // note the colon

Vector::Vector(){ x=0; y=0; }

Vector::Vector() : x(0), y(0){}

Vector::Vector(const Vector &source) {
x = (source.x)*2 ; y = (source.y)*2 ;
}

Vector::Vector( Vector const &source) :
x( (source.x)*2 ), y( (source.y)*2 ){}

Vector Vector:perator+ (const Vector &a) const {
Vector temp;
temp.x = x + a.x;
temp.y = y + a.y;
return (temp);
}

int main() {
Vector a(3,1), b(5,2), c, d;
c = (a+b);
d = (a.operator+ (b));
cout << "Data members of the vector c: ";
c.Show();
Vector e((a+b));
cout <<endl << "Data members of the vector e: ";
e.Show();

return 0;
}

Did you use g++ ? Because now i'm getting (8,3) for both c and e.

GCC(MinGW)3.3.1 g++, I got:

// out: Data members of the vector c: (8.000000,3.000000)
// out: Data members of the vector e: (8.000000,3.000000)

Thanks for the patience and for the (void) tip.

Please don't top-post.

 

[I V]

IV, how can i improve this? I came from a little C background and i
just learned how to do operator overloading.

The obvious copy constructor would be:

Vector::Vector(const Vector &source) {
x = source.x ; y = source.y;
}

(or use initialization lists, as BobR suggests, or just let the compiler
generate the copy-constructor).

This would clearly give you different results, namely (8, 3) rather than
(16, 6). I'm not sure exactly what you're trying to accomplish; why are you
multiplying the x and y elements of your vector by 2 when you make a copy?

 

[Jim Langston]

Jim, did you mean something like this?

#include <iostream>
#include <string>
using namespace std;

class Vector {
private:
float x,y;
public:
Vector(float u, float v);
Vector();
Vector operator+ (const Vector &a) const;
Vector(const Vector &source);
void Show();
};
void Vector::Show() {
cout <<"(" << x <<"," <<y <<")";
}
Vector::Vector(float u, float v) {
x=u; y=v;
}
Vector::Vector() {
x=0; y=0;
}
Vector::Vector(const Vector &source) {
x = (source.x)*2 ; y = (source.y)*2 ;
}
Vector Vector:perator+ (const Vector &a) const {
Vector temp;
temp.x = x + a.x;
temp.y = y + a.y;
return (temp);
}

int main() {
Vector a(3,1), b(5,2), c, d;
c = (a+b);
d = (a.operator+ (b));
cout << "Data members of the vector c: ";
c.Show();
Vector e((a+b));
cout <<endl << "Data members of the vector e: ";
e.Show();

return 0;
}

Did you use g++ ? Because now i'm getting (8,3) for both c and e.

Thanks for the patience and for the (void) tip.

Here, your "copy constructor" doesn't make an actual copy. Are you sure
you
want to do that?

IV, how can i improve this? I came from a little C background and i
just learned how to do operator overloading.

A simple program with operator overloading and copy constructor: [...]
Vector::Vector(Vector &source) {
x = (source.x)*2 ; y = (source.y)*2 ;
}

Here, your "copy constructor" doesn't make an actual copy. Are you sure
you
want to do that? It seems to me to be likely to cause confusion. Indeed,
I
think this is why you're not getting the result you expect:

Vector a(3,1), b(5,2), c, d;
c = a+b;

Now, this calls a.operator+(b), which invokes the copy constructor on b;
so a.operator+ gets passed (10, 4); it then adds this to (3, 1), giving
(13, 5), and returns this, which itself invokes the copy constructor,
giving (26, 10). However, I think this last use of the copy constructor,
in the return, is optional; so the results of calling operator+ depend on
whether or not the compiler decides to optimize out the copy constructor.
This doesn't seem like a very good state of affairs.

Here's your program cleaned up a bit and extened a little.

#include <iostream>
#include <string>

class Vector
{
private:
float x,y;
public:
Vector(float u, float v);
Vector operator+ (const Vector &a) const;
Vector operator* (const float a ) const;
Vector operator* (const Vector &b) const;
Vector(const Vector &source);
void Show();
};

void Vector::Show()
{
std::cout <<"(" << x <<"," <<y <<")";
}

Vector::Vector(float u = 0, float v = 0): x(u), y(v)
{
}


Vector::Vector(const Vector &source)
{
x = source.x;
y = source.y;
}

Vector Vector:perator+ (const Vector &a) const
{
Vector temp;
temp.x = x + a.x;
temp.y = y + a.y;
return temp;
}

Vector Vector:perator* (float a) const
{
Vector temp;
temp.x = x * a;
temp.y = y * a;
return temp;
}

Vector Vector:perator* (const Vector& a ) const
{
Vector temp;
temp.x = x * a.x;
temp.y = y * a.y;
return temp;
}

int main()
{
Vector a(3,1), b(5,2), c, d;

c = a+b;
d = (a.operator+ (b));
std::cout << "Data members of the vector c: ";
c.Show();
Vector e((a+b));
std::cout << std::endl << "Data members of the vector e: ";
e.Show();
std::cout << "\na * 2 = ";
(a * 2).Show();
std::cout << "\na * b = ";
(a * b).Show();

return 0;
}

All a copy constructor should do is copy. x = x, y = y. Nothing more. If
you want to mulitply, have an operator*

The multiplication of the two Vectors (Vector * Vector) is arbitary. A lot
of times multiplying two vectors would return the dot product. It dpends on
what you want it to do.

 

[clicwar]

All a copy constructor should do is copy. x = x, y = y. Nothing more. If

you want to mulitply, have an operator*

Now i understand. I was looking to the copy operator as a way to do
whatever i want with the data members in the moment of the
instantiation of the object.
Now i know that the copy constructor just ... copy.
If i want an additional functionality , i must supply an additional
function outside the copy constructor.

At this moment the fact of implement my own copy constructors seems
very useless, since the compiler provides me a default implementation.
But i'm sure that in the future i will change my mind.

Please don't top-post

Sorry, i don't know what is top-post. But, surely i will not do again
if you tell me what it is.

Thanks Jim,IV and BobR.

Jim, did you mean something like this?

#include <iostream>
#include <string>
using namespace std;

class Vector {
private:
float x,y;
public:
Vector(float u, float v);
Vector();
Vector operator+ (const Vector &a) const;
Vector(const Vector &source);
void Show();
};
void Vector::Show() {
cout <<"(" << x <<"," <<y <<")";
}
Vector::Vector(float u, float v) {
x=u; y=v;
}
Vector::Vector() {
x=0; y=0;
}
Vector::Vector(const Vector &source) {
x = (source.x)*2 ; y = (source.y)*2 ;
}
Vector Vector:perator+ (const Vector &a) const {
Vector temp;
temp.x = x + a.x;
temp.y = y + a.y;
return (temp);
}

int main() {
Vector a(3,1), b(5,2), c, d;
c = (a+b);
d = (a.operator+ (b));
cout << "Data members of the vector c: ";
c.Show();
Vector e((a+b));
cout <<endl << "Data members of the vector e: ";
e.Show();

return 0;
}

Did you use g++ ? Because now i'm getting (8,3) for both c and e.

Thanks for the patience and for the (void) tip.
IV, how can i improve this? I came from a little C background and i
just learned how to do operator overloading.

[...]
Vector::Vector(Vector &source) {
x = (source.x)*2 ; y = (source.y)*2 ;
}
Here, your "copy constructor" doesn't make an actual copy. Are you sure
you
want to do that? It seems to me to be likely to cause confusion. Indeed,
I
think this is why you're not getting the result you expect:
Vector a(3,1), b(5,2), c, d;
c = a+b;
Now, this calls a.operator+(b), which invokes the copy constructor on b;
so a.operator+ gets passed (10, 4); it then adds this to (3, 1), giving
(13, 5), and returns this, which itself invokes the copy constructor,
giving (26, 10). However, I think this last use of the copy constructor,
in the return, is optional; so the results of calling operator+ depend on
whether or not the compiler decides to optimize out the copy constructor.
This doesn't seem like a very good state of affairs.

Here's your program cleaned up a bit and extened a little.

#include <iostream>
#include <string>

class Vector
{
private:
float x,y;
public:
Vector(float u, float v);
Vector operator+ (const Vector &a) const;
Vector operator* (const float a ) const;
Vector operator* (const Vector &b) const;
Vector(const Vector &source);
void Show();

};

void Vector::Show()
{
std::cout <<"(" << x <<"," <<y <<")";

}

Vector::Vector(float u = 0, float v = 0): x(u), y(v)
{

}

Vector::Vector(const Vector &source)
{
x = source.x;
y = source.y;

}

Vector Vector:perator+ (const Vector &a) const
{
Vector temp;
temp.x = x + a.x;
temp.y = y + a.y;
return temp;

}

Vector Vector:perator* (float a) const
{
Vector temp;
temp.x = x * a;
temp.y = y * a;
return temp;

}

Vector Vector:perator* (const Vector& a ) const
{
Vector temp;
temp.x = x * a.x;
temp.y = y * a.y;
return temp;

}

int main()
{
Vector a(3,1), b(5,2), c, d;

c = a+b;
d = (a.operator+ (b));
std::cout << "Data members of the vector c: ";
c.Show();
Vector e((a+b));
std::cout << std::endl << "Data members of the vector e: ";
e.Show();
std::cout << "\na * 2 = ";
(a * 2).Show();
std::cout << "\na * b = ";
(a * b).Show();

return 0;

}

All a copy constructor should do is copy. x = x, y = y. Nothing more. If
you want to mulitply, have an operator*

The multiplication of the two Vectors (Vector * Vector) is arbitary. A lot
of times multiplying two vectors would return the dot product. It dpends on
what you want it to do.

 

[Jim Langston]

Jim, did you mean something like this?

#include <iostream>
#include <string>
using namespace std;

class Vector {
private:
float x,y;
public:
Vector(float u, float v);
Vector();
Vector operator+ (const Vector &a) const;
Vector(const Vector &source);
void Show();
};
void Vector::Show() {
cout <<"(" << x <<"," <<y <<")";
}
Vector::Vector(float u, float v) {
x=u; y=v;
}
Vector::Vector() {
x=0; y=0;
}
Vector::Vector(const Vector &source) {
x = (source.x)*2 ; y = (source.y)*2 ;
}
Vector Vector:perator+ (const Vector &a) const {
Vector temp;
temp.x = x + a.x;
temp.y = y + a.y;
return (temp);
}

int main() {
Vector a(3,1), b(5,2), c, d;
c = (a+b);
d = (a.operator+ (b));
cout << "Data members of the vector c: ";
c.Show();
Vector e((a+b));
cout <<endl << "Data members of the vector e: ";
e.Show();

return 0;
}

Did you use g++ ? Because now i'm getting (8,3) for both c and e.

Thanks for the patience and for the (void) tip.

Here, your "copy constructor" doesn't make an actual copy. Are you
sure
you
want to do that?
IV, how can i improve this? I came from a little C background and i
just learned how to do operator overloading.

[...]
Vector::Vector(Vector &source) {
x = (source.x)*2 ; y = (source.y)*2 ;
}

Here, your "copy constructor" doesn't make an actual copy. Are you
sure
you
want to do that? It seems to me to be likely to cause confusion.
Indeed,
I
think this is why you're not getting the result you expect:

Vector a(3,1), b(5,2), c, d;
c = a+b;

Now, this calls a.operator+(b), which invokes the copy constructor on
b;
so a.operator+ gets passed (10, 4); it then adds this to (3, 1),
giving
(13, 5), and returns this, which itself invokes the copy constructor,
giving (26, 10). However, I think this last use of the copy
constructor,
in the return, is optional; so the results of calling operator+ depend
on
whether or not the compiler decides to optimize out the copy
constructor.
This doesn't seem like a very good state of affairs.

Here's your program cleaned up a bit and extened a little.

#include <iostream>
#include <string>

class Vector
{
private:
float x,y;
public:
Vector(float u, float v);
Vector operator+ (const Vector &a) const;
Vector operator* (const float a ) const;
Vector operator* (const Vector &b) const;
Vector(const Vector &source);
void Show();

};

void Vector::Show()
{
std::cout <<"(" << x <<"," <<y <<")";

}

Vector::Vector(float u = 0, float v = 0): x(u), y(v)
{

}

Vector::Vector(const Vector &source)
{
x = source.x;
y = source.y;

}

Vector Vector:perator+ (const Vector &a) const
{
Vector temp;
temp.x = x + a.x;
temp.y = y + a.y;
return temp;

}

Vector Vector:perator* (float a) const
{
Vector temp;
temp.x = x * a;
temp.y = y * a;
return temp;

}

Vector Vector:perator* (const Vector& a ) const
{
Vector temp;
temp.x = x * a.x;
temp.y = y * a.y;
return temp;

}

int main()
{
Vector a(3,1), b(5,2), c, d;

c = a+b;
d = (a.operator+ (b));
std::cout << "Data members of the vector c: ";
c.Show();
Vector e((a+b));
std::cout << std::endl << "Data members of the vector e: ";
e.Show();
std::cout << "\na * 2 = ";
(a * 2).Show();
std::cout << "\na * b = ";
(a * b).Show();

return 0;

}

All a copy constructor should do is copy. x = x, y = y. Nothing more.
If
you want to mulitply, have an operator*

The multiplication of the two Vectors (Vector * Vector) is arbitary. A
lot
of times multiplying two vectors would return the dot product. It dpends
on
what you want it to do.

All a copy constructor should do is copy. x = x, y = y. Nothing more.
If
you want to mulitply, have an operator*

Now i understand. I was looking to the copy operator as a way to do
whatever i want with the data members in the moment of the
instantiation of the object.
Now i know that the copy constructor just ... copy.
If i want an additional functionality , i must supply an additional
function outside the copy constructor.

At this moment the fact of implement my own copy constructors seems
very useless, since the compiler provides me a default implementation.
But i'm sure that in the future i will change my mind.

Please don't top-post

Sorry, i don't know what is top-post. But, surely i will not do again
if you tell me what it is.

Thanks Jim,IV and BobR.

Message rearranged to be proper. This is how to post, put your replies
AFTER what you're replying to, not before. If you reply before, it is known
as "top posting" and is considered bad in this, and many, newsgroups.

But, yes, a lot of the time the default construction will work fine for you.
Exceptions are when you have a pointer method, and need to copy the pointer
not the data.

 

[BobR]

Now i understand. I was looking to the copy operator as a way to do
whatever i want with the data members in the moment of the
instantiation of the object.
Now i know that the copy constructor just ... copy.
If i want an additional functionality , i must supply an additional
function outside the copy constructor.

At this moment the fact of implement my own copy constructors seems
very useless, since the compiler provides me a default implementation.
But i'm sure that in the future i will change my mind.

As soon as you add a pointer to dynamic allocated memory (new) in your
class/struct, you *really need* a copy constructor (or prevent copy). There
are other things that scream, "Hey, I NEED a copy constructor or I'll throw
you a curve you won't soon forget!".

Take a trip through this FAQ for some good info, tips, 'gotchas', etc.
FAQ http://www.parashift.com/c++-faq-lite

Sorry, i don't know what is top-post. But, surely i will not do again
if you tell me what it is.

While your in the FAQ, don't forget the "how to post" sections. You didn't
top-post this time, but close to it. :-}

As Alf P. Steinbach puts it:
--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

Thanks Jim,IV and BobR.

Your welcome.

--
Bob R
POVrookie
- -
Get "Thinking in C++", 2nd ed. Volume 1&2 by Bruce Eckel
(available for free here. You can buy it in hardcopy too.):
http://www.mindview.net/Books/TICPP/ThinkingInCPP2e.html

 

[Jim Langston]

[Snip unrelated to correct]

But, yes, a lot of the time the default construction will work fine for
you. Exceptions are when you have a pointer method, and need to copy the
pointer not the data.

Ack, I meant you'd need a custom copy constructor (and assignment operator)
if need to copy the pointer data, not the pointer itself. Also, the "Rule
of Three" applies. Before you ask what the Rule of Three is, google or
check the FAQ

 

[James Kanze]

[I've reordered the quoting some, so that my answers make
some sense. I hope I didn't screw up in doing it.]


[...]
Note the semantics of the copy constructor. This doesn't copy.

And note that the operator+ takes its argument by copy.

[...]
You can't bind a temporary to a non-const reference. With your
original version of the copy constructor, you cannot copy
temporary values, only named variables.

If you fail to define a copy constructor, the compiler generates
one for you automatically. Which has a const reference as
parameter. (The rules are actually a little more complicated,
but that's the usual situation.)

Well, I get even a weirder result with your code (26.3 or
something). Not sure why,

His copy constructor doesn't copy; the new object has different
values. The compiler is allowed to assume that a copy
constructor copies, and in various cases, suppress intermediate
temporaries, and the copy that was used to create them. Given
his copy constructor, a large range of results are possible.

but if you fix your operator+ it'll show the correct answer of
16.6

Vector operator+ (const Vector& a) const;

Operators and constructors have specific signatures.

Only a very few do. For the most part, all that is required is
that they take the right number of arguments, and that at least
one parameter is a user defined type (class or enum). There's
absolutely nothing wrong with his operator+ (from a language
standpoint---it certainly isn't very idiomatic); the problem is
with his copy constructor.

const is part of them and I believe references are also.

I don't think that there's any case where the const is required
by the language (although the operator may not be usable where
one would expect to use it without the const), and about the
only time a reference is required is for the copy constructor.

That's from the language standpoint, of course. From the point
of view of usability, or user expectations, you really want to
follow the usual models. (But even in the usual models, whether
operator+ takes a const reference or a value depends on the
type.)

I really don't know what was going on that was giving us the
wrong results (I.E. no clue what the compiler was donig) and
don't really care, cause fixing the operator+ fixes the
problem.

As to why it didn't work without your constructor having
const, that's because it didn't recognize it as a copy
constructor.

If the compiler hadn't recognized his constructor as a copy
constructor, it would have generated one, and his original code
would have compiled. The problem is just the opposite; his code
didn't work because the compiler recognized that he had already
provided a copy constructor. Even though it couldn't be
called in a number of situations because it didn't have the
"usual" signature, the compiler didn't generate another one.

Which makes me think, changing it from a copy to a reference
probably means your copy constructor is not working right, let
me look at it...

Exactly. And you have to be careful with copy constructors,
because the compiler is allowed to assume that all they do is
copy; the number of times it actually gets called may vary from
one compiler to the next, or according to the optimization
options.

 

[James Kanze]

news:[email protected]...

[...]

Here's your program cleaned up a bit and extened a little.

Cleaned up still some more, to make it more idiomatic. (The
changes don't actually make a difference here, but in many
cases, they do, and it's good practice to stick to.)

#include <iostream>
#include <string>

class Vector
{
private:
float x,y;

Normally, of course, we'd use double everywhere. Float is
reserved for cases where we have to save memory.

public:
Vector(float u, float v);
Vector operator+ (const Vector &a) const;

Most of the time, it is preferable to make operator+ a free
function. It only really makes a difference if implicit
conversions are involved, but it's never wrong, so why not be
consistent about it.

Also, the most important single rule about operator overloading
is to do it consistently with what the built in types do. So if
you define +, you also define +=. With the same semantics.
It's fairly frequent to define operator+ in terms of +=, e.g.:

// Free function...
Vector operator+( Vector lhs, Vector rhs )
{
lhs += rhs ;
return lhs ;
}

or
Vector operator+( Vector const& lhs, Vector const& rhs )
{
Vector result( lhs ) ;
result += rhs ;
return result ;
}

(The generation of these free functions can actually be
automated pretty well by means of some clever use of templates,
but I don't think that the original poster is ready for that
yet.)

Vector operator* (const float a ) const;
Vector operator* (const Vector &b) const;
Vector(const Vector &source);
void Show();
};

void Vector::Show()
{
std::cout <<"(" << x <<"," <<y <<")";
}

Vector::Vector(float u = 0, float v = 0): x(u), y(v)
{
}

Given that this function will typically be in a separate soruce
file, the default arguments don't make any sense here. They
should be on the function declaration in the class.

And if you use them, you've created a converting constructor,
and you'll definitely want operator+ to be a non-member:

Vector a, b ;
a = b + 3.14 ; // Legal, member or not...
a = 3.14 + b ; // Legal if non-member, illegal if member.

Vector::Vector(const Vector &source)
{
x = source.x;
y = source.y;
}

Again, it makes no difference in this particular case, but you
really should prefer initialization lists:

Vector::Vector(
Vector const& source )
: x( source.x )
, y( source.y )
{
}

(And while I'm at it: note the position of the const. In this
case, const Vector and Vector const are both legal, but most of
the time, the const must follow what it modifies, so for
consistency's sake, we always put it after.)

Vector Vector:perator+ (const Vector &a) const
{
Vector temp;
temp.x = x + a.x;
temp.y = y + a.y;
return temp;
}

Vector Vector:perator* (float a) const
{
Vector temp;
temp.x = x * a;
temp.y = y * a;
return temp;
}

So you support "v * 3.14", but not "3.14 * v". Not good, IMHO.

 

[clicwar]

[...]

Here's your program cleaned up a bit and extened a little.

Cleaned up still some more, to make it more idiomatic. (The
changes don't actually make a difference here, but in many
cases, they do, and it's good practice to stick to.)

#include <iostream>
#include <string>

class Vector
{
private:
float x,y;

Normally, of course, we'd use double everywhere. Float is
reserved for cases where we have to save memory.

public:
Vector(float u, float v);
Vector operator+ (const Vector &a) const;

Most of the time, it is preferable to make operator+ a free
function. It only really makes a difference if implicit
conversions are involved, but it's never wrong, so why not be
consistent about it.

Also, the most important single rule about operator overloading
is to do it consistently with what the built in types do. So if
you define +, you also define +=. With the same semantics.
It's fairly frequent to define operator+ in terms of +=, e.g.:

// Free function...
Vector operator+( Vector lhs, Vector rhs )
{
lhs += rhs ;
return lhs ;
}

or
Vector operator+( Vector const& lhs, Vector const& rhs )
{
Vector result( lhs ) ;
result += rhs ;
return result ;
}

(The generation of these free functions can actually be
automated pretty well by means of some clever use of templates,
but I don't think that the original poster is ready for that
yet.)

Vector operator* (const float a ) const;
Vector operator* (const Vector &b) const;
Vector(const Vector &source);
void Show();
};
void Vector::Show()
{
std::cout <<"(" << x <<"," <<y <<")";
}
Vector::Vector(float u = 0, float v = 0): x(u), y(v)
{
}

Given that this function will typically be in a separate soruce
file, the default arguments don't make any sense here. They
should be on the function declaration in the class.

And if you use them, you've created a converting constructor,
and you'll definitely want operator+ to be a non-member:

Vector a, b ;
a = b + 3.14 ; // Legal, member or not...
a = 3.14 + b ; // Legal if non-member, illegal if member.

Vector::Vector(const Vector &source)
{
x = source.x;
y = source.y;
}

Again, it makes no difference in this particular case, but you
really should prefer initialization lists:

Vector::Vector(
Vector const& source )
: x( source.x )
, y( source.y )
{
}

(And while I'm at it: note the position of the const. In this
case, const Vector and Vector const are both legal, but most of
the time, the const must follow what it modifies, so for
consistency's sake, we always put it after.)

Vector Vector:perator+ (const Vector &a) const
{
Vector temp;
temp.x = x + a.x;
temp.y = y + a.y;
return temp;
}
Vector Vector:perator* (float a) const
{
Vector temp;
temp.x = x * a;
temp.y = y * a;
return temp;
}

So you support "v * 3.14", but not "3.14 * v". Not good, IMHO.

--
James Kanze (GABI Software) email:[email protected]
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34

James Kanze, thank you for your time.
After re-re-read your posts, i have to confess that i re-confused
myself again.

I concentrate this confusion in two quick problems. The code below
will help to make the questions make sense:

#include <iostream>
using namespace std;

class Vector {
private:
float x,y;
public:
Vector(float u, float v);
Vector();
Vector operator+ (Vector const &a);
Vector(Vector &source);
void Show();
};
void Vector::Show() {
cout <<"(" << x <<"," <<y <<")";
}
Vector::Vector(float u, float v) {
x=u; y=v;
}
Vector::Vector() {
x=0; y=0;
}
Vector::Vector(Vector &source) { //this is the line24
x = (source.x) ; y=10;
}
Vector Vector:perator+ (Vector const &a) {
Vector temp;
temp.x = x + a.x;
temp.y = y + a.y;
return (temp);
}

int main() {
Vector a(3,1), b(5,2), c(a), d;
d = (a.operator+ (b)); //this is the line36
cout <<endl<< "Data members of the vector c: ";
c.Show();
cout <<endl<< "Data members of the vector d: ";
d.Show();
Vector e = b;
cout <<endl<< "Data members of the vector e: ";
e.Show();
Vector f(a);
cout <<endl<< "Data members of the vector f: ";
f.Show();

return 0;
}

And the compiler(g++) says:
teste.cpp: In function `int main()':
teste.cpp:36: error: no matching function for call to
`Vector::Vector(Vector)'
teste.cpp:24: note: candidates are: Vector::Vector(Vector&)


1º)

Exactly. And you have to be careful with copy constructors,
because the compiler is allowed to assume that all they do is
copy; the number of times it actually gets called may vary from
one compiler to the next, or according to the optimization
options.

So... do you agree that if i want to do any other kind of things
in the instantion of the object , i must provide additional methods
and not use the copy constructor, not even for an innocent y=10 or
y=(source.y)*2 ???


2º)

You can't bind a temporary to a non-const reference. With your
original version of the copy constructor, you cannot copy
temporary values, only named variables.

I'm not sure if i get this recommendation and i think this is the key
concept that i'm missing, that keeps me confused.
Could please write an example?


This is how i'm seeing the problem:

If i change the line36 from
d = (a.operator+ (b)); //this is the line36
to
(a.operator+ (b)); //this is the line36
the program compiles and runs. But i'm not using anymore temporary
values with the copy constructor. Now i'm using temporary values (the
temp object returned by the operator+ method) with the assignment
operator.
If i remove the assignment operation, the problem is gone.
So what's wrong with the assignment operator?

OR
if i change the definition(and declaration) of the constructor from
Vector(Vector &source);
to
Vector(Vector const &source);
the problem is gone again. But how can the copy constructor interferes
with the assignment operation of line36 ??


Thanks.

 

[=?ISO-8859-1?Q?Erik_Wikstr=F6m?=]

[...]

Here's your program cleaned up a bit and extened a little.

Cleaned up still some more, to make it more idiomatic. (The
changes don't actually make a difference here, but in many
cases, they do, and it's good practice to stick to.)

#include <iostream>
#include <string>

class Vector
{
private:
float x,y;

Normally, of course, we'd use double everywhere. Float is
reserved for cases where we have to save memory.

public:
Vector(float u, float v);
Vector operator+ (const Vector &a) const;

Most of the time, it is preferable to make operator+ a free
function. It only really makes a difference if implicit
conversions are involved, but it's never wrong, so why not be
consistent about it.

Also, the most important single rule about operator overloading
is to do it consistently with what the built in types do. So if
you define +, you also define +=. With the same semantics.
It's fairly frequent to define operator+ in terms of +=, e.g.:

// Free function...
Vector operator+( Vector lhs, Vector rhs )
{
lhs += rhs ;
return lhs ;
}

or
Vector operator+( Vector const& lhs, Vector const& rhs )
{
Vector result( lhs ) ;
result += rhs ;
return result ;
}

(The generation of these free functions can actually be
automated pretty well by means of some clever use of templates,
but I don't think that the original poster is ready for that
yet.)

Vector operator* (const float a ) const;
Vector operator* (const Vector &b) const;
Vector(const Vector &source);
void Show();
};
void Vector::Show()
{
std::cout <<"(" << x <<"," <<y <<")";
}
Vector::Vector(float u = 0, float v = 0): x(u), y(v)
{
}

Given that this function will typically be in a separate soruce
file, the default arguments don't make any sense here. They
should be on the function declaration in the class.

And if you use them, you've created a converting constructor,
and you'll definitely want operator+ to be a non-member:

Vector a, b ;
a = b + 3.14 ; // Legal, member or not...
a = 3.14 + b ; // Legal if non-member, illegal if member.

Vector::Vector(const Vector &source)
{
x = source.x;
y = source.y;
}

Again, it makes no difference in this particular case, but you
really should prefer initialization lists:

Vector::Vector(
Vector const& source )
: x( source.x )
, y( source.y )
{
}

(And while I'm at it: note the position of the const. In this
case, const Vector and Vector const are both legal, but most of
the time, the const must follow what it modifies, so for
consistency's sake, we always put it after.)

Vector Vector:perator+ (const Vector &a) const
{
Vector temp;
temp.x = x + a.x;
temp.y = y + a.y;
return temp;
}
Vector Vector:perator* (float a) const
{
Vector temp;
temp.x = x * a;
temp.y = y * a;
return temp;
}

So you support "v * 3.14", but not "3.14 * v". Not good, IMHO.

--
James Kanze (GABI Software) email:[email protected]
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34

James Kanze, thank you for your time.
After re-re-read your posts, i have to confess that i re-confused
myself again.

I concentrate this confusion in two quick problems. The code below
will help to make the questions make sense:

#include <iostream>
using namespace std;

class Vector {
private:
float x,y;
public:
Vector(float u, float v);
Vector();
Vector operator+ (Vector const &a);
Vector(Vector &source);
void Show();
};
void Vector::Show() {
cout <<"(" << x <<"," <<y <<")";
}
Vector::Vector(float u, float v) {
x=u; y=v;
}
Vector::Vector() {
x=0; y=0;
}
Vector::Vector(Vector &source) { //this is the line24
x = (source.x) ; y=10;
}
Vector Vector:perator+ (Vector const &a) {
Vector temp;
temp.x = x + a.x;
temp.y = y + a.y;
return (temp);
}

int main() {
Vector a(3,1), b(5,2), c(a), d;
d = (a.operator+ (b)); //this is the line36
cout <<endl<< "Data members of the vector c: ";
c.Show();
cout <<endl<< "Data members of the vector d: ";
d.Show();
Vector e = b;
cout <<endl<< "Data members of the vector e: ";
e.Show();
Vector f(a);
cout <<endl<< "Data members of the vector f: ";
f.Show();

return 0;
}

And the compiler(g++) says:
teste.cpp: In function `int main()':
teste.cpp:36: error: no matching function for call to
`Vector::Vector(Vector)'
teste.cpp:24: note: candidates are: Vector::Vector(Vector&)

You can't bind a temporary to a reference, you need to make the
parameter to the copy-constructor a const reference:

Vector::Vector(const Vector& source);

1º)
So... do you agree that if i want to do any other kind of things
in the instantion of the object , i must provide additional methods
and not use the copy constructor, not even for an innocent y=10 or
y=(source.y)*2 ???

Yes, you should use a normal constructor and pass parameters telling it
what kind of behaviour you expect, perhaps something like

Vector::Vector(float x, float y);

As the name copy-constructor implies it creates a copy of the object,
even if the compiler did not make any assumptions about it other
programmers will, they expect to get a perfect copy out of it , nothing
else.

2º)
I'm not sure if i get this recommendation and i think this is the key
concept that i'm missing, that keeps me confused.
Could please write an example?

I'm not 100% sure on the details, but here's an attempt at explaining:
Your own code above, the on the line 'd = (a.operator+ (b));' operator+
will return a new Vector object, which will then be assigned to d, but
to avoid unnecessary copying the compiler is allowed to optimise away
the assignment and copy-construct the return value directly into d,
instead of first creating a temporary. To do that the copy-constructor
have to be able to create a copy from a temporary object.

 

[James Kanze]

On 23 jul, 06:35, James Kanze <[email protected]> wrote:

[...]

I concentrate this confusion in two quick problems. The code below
will help to make the questions make sense:

#include <iostream>
using namespace std;

class Vector {
private:
float x,y;
public:
Vector(float u, float v);
Vector();
Vector operator+ (Vector const &a);
Vector(Vector &source);

Attention. This is a copy constructor. It will prevent the
compiler from generating a default copy constructor. On the
other hand, it will NOT copy const objects or temporaries. It
that really what you want?

[...]

d = (a.operator+ (b)); //this is the line36

[...]

And the compiler(g++) says:
teste.cpp: In function `int main()':
teste.cpp:36: error: no matching function for call to
`Vector::Vector(Vector)'
teste.cpp:24: note: candidates are: Vector::Vector(Vector&)

This is a bit tricky, but in this line, you're invoking the
assignment operator. The default assignment operator, since you
haven't provided one. The default assignment operator has a
const reference as its parameter, so you are binding a temporary
(the return value of operator+) to a const reference. The
language specification says that to do this, the temporary must
be copiable. Your copy constructor doesn't support copying
temporaries, so the compiler complains.

You almost certainly want the copy constructor here to take a
const reference. Or to just leave it to the compiler.

1º)>Exactly. And you have to be careful with copy constructors,

So... do you agree that if i want to do any other kind of things
in the instantion of the object , i must provide additional methods
and not use the copy constructor, not even for an innocent y=10 or
y=(source.y)*2 ???

The copy constructor should copy. You need to be able to copy.
If you need other functionality as well, then you have to
provide other functions and/or constructors to provide them.
The copy constructor is taken.

2º)>You can't bind a temporary to a non-const reference. With your

I'm not sure if i get this recommendation and i think this is the key
concept that i'm missing, that keeps me confused.

It's not a recommendation, it's the law. The language
specification (ISO 14882) says that to initialize a reference,
the initialization argument must be an lvalue (or implicitly
convertable to an lvalue), or the reference must be to const non
volatile. If the function takes a T&, then the argument must be
an lvalue---roughly speaking, something which has an address.
The return value of a function doesn't have an address (at least
conceptually), is a temporary, and cannot be bound to a
non-const reference.

Could please write an example?

void f( int const& ) ;
void g( int& ) ;

// ...

f( 3 + 5 ) ; // Legal...
g( 3 + 5 ) ; // Illegal...

This is how i'm seeing the problem:

If i change the line36 from
d = (a.operator+ (b)); //this is the line36
to
(a.operator+ (b)); //this is the line36
the program compiles and runs.

Of course. You're not trying to bind the return value of
operator+ to a reference. (You're not doing anything with it,
in fact.)

But i'm not using anymore temporary values with the copy
constructor. Now i'm using temporary values (the temp object
returned by the operator+ method) with the assignment
operator.

The rules are a bit complicated, but suffice it to say that:

1. the compiler generated default assignment operator takes a
reference,

2. the standard says that to bind a temporary to a reference,
the temporary must be copiable (even if, normally, the
compiler does not actually make a copy), and

3. a copy constructor which takes a non-const reference cannot
copy a temporary.

If i remove the assignment operation, the problem is gone.
So what's wrong with the assignment operator?

Nothing. The actual problem is with your copy constructor.
Now, it doesn't make sense to make a copy here, and no compiler
I know will actually call the copy constructor. But the
standard says that the object must be copiable, or the code is
illegal, and your copy constructor won't copy a temporary, so
you're out of luck.

The situation is made worse by the fact that many (most?)
compilers don't enforce this rule, so people tend to overlook it
in real code, and get bitten when porting to a stricter
compiler. You're lucky; the compiler you are learning on does
enforce it, so you're forced to learn the language correctly.

OR
if i change the definition(and declaration) of the constructor from
Vector(Vector &source);
to
Vector(Vector const &source);
the problem is gone again. But how can the copy constructor interferes
with the assignment operation of line36 ??

Good question. How can it? All I can say is that the standard
requires it, even if it doesn't make sense.

(In fact, there are some cases where it does make sense, and
rather than require it sometimes, and not others, the standard
opted to be orthogonal. But the cases are rather complex, and
it's probably better at your level just to think of it as an
arbitrary rule. You'll have to learn enough of those anyway;
one more won't hurt you.)

 

[James Kanze]

I'm not 100% sure on the details,

The details are simple: the standard says that an implementation
may copy, even if it doesn't make sense, and the standard
requires that the value being assigned by copiable, even if
there's absolutely no need for a copy in reality.

but here's an attempt at explaining:
Your own code above, the on the line 'd = (a.operator+ (b));' operator+
will return a new Vector object, which will then be assigned to d, but
to avoid unnecessary copying the compiler is allowed to optimise away
the assignment and copy-construct the return value directly into d,

Never. Imagine that d had a pointer to dynamically allocated
memory. Are you saying that the compiler should be allowed to
copy construct into that memory, overwriting the pointer to
dynamically allocated memory (and thus creating a memory leak).
There are two absolute invariants in the language: an object is
constructed exactly once, and it is destructed exactly once.
The compiler will never, ever generate a call to a constructor
on an already constructed object. (Barring compiler errors, of
course, but such errors have become very, very rare.)

instead of first creating a temporary. To do that the
copy-constructor have to be able to create a copy from a
temporary object.

The problem has nothing to do with the creation of the
temporary. The problem is binding the temporary to the
reference parameter of operator=.

 

[clicwar]

The details are simple: the standard says that an implementation
may copy, even if it doesn't make sense, and the standard
requires that the value being assigned by copiable, even if
there's absolutely no need for a copy in reality.


Never. Imagine that d had a pointer to dynamically allocated
memory. Are you saying that the compiler should be allowed to
copy construct into that memory, overwriting the pointer to
dynamically allocated memory (and thus creating a memory leak).
There are two absolute invariants in the language: an object is
constructed exactly once, and it is destructed exactly once.
The compiler will never, ever generate a call to a constructor
on an already constructed object. (Barring compiler errors, of
course, but such errors have become very, very rare.)


The problem has nothing to do with the creation of the
temporary. The problem is binding the temporary to the
reference parameter of operator=.

--
James Kanze (GABI Software) email:[email protected]
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34

I finally can make the logic connections in my mind.
Thankyou very much, James and all the other who spent their valuable
time in this topic.

James,just one more question. Could you recommend me a book tutorial/
site which teaches c++ using this deep approach, or such book does not
exist, at least for the beginner public ??