Something funny happening with operator overloading

The arithmetics of the program are incorrect
mvector.h

Code:

#include <cmath>
#ifndef _MVECTOR_H
#define _MVECTOR_H
namespace math
{
class Vector
{
    public:
        float x;
        float y;
        float z;

        Vector(float a=0,float b=0,float c=0)
        {
            x=a;
            y=b;
            z=c;
        }


    Vector operator+(Vector &v1)
    {
        Vector tmp;
           tmp.x=v1.x+x;
        tmp.y=v1.y+y;
        tmp.z=v1.z+z;
            return tmp;
    };

    Vector operator-(Vector v1)
    {
        Vector tmp;
        tmp.x=x-v1.x;
        tmp.y=y-v1.y;
        tmp.y=z-v1.z;

            return tmp;
    };


float VecLength(Vector v)
{
    return sqrt(pow(v.x,2)+pow(v.y,2)+pow(v.z,2));
};

float DotProduct(Vector v1,Vector v2)
{
    return (v1.x*v2.x + v1.y*v2.y + v1.z*v2.z);
}
    Vector CrossProduct(Vector v1, Vector v2)
    {
        Vector temp;
        temp.x=v1.y*v2.z - v1.z*v2.y;
        temp.y=v1.z*v2.x - v1.x*v2.z;
        temp.z=v1.x*v2.y - v1.y*v2.x;
        return temp;
    };
};
}
#endif

main.cpp

Code:

#include <iostream>
#include <cmath>
#include "Mvector.h"
using namespace math;
using namespace std;
Vector a= Vector(1,-8,-3);
Vector b=Vector(2,2,3);

Vector c= a-b;

int main()
{
	std::cout <<c.x<< std::endl;
	std::cout <<c.y<< std::endl;
	std::cout <<c.z<< std::endl;
	return 0;
}

for some reason it outputs:
-1
-6
0
whats wrong?

Code:

        tmp.x=x-v1.x;
        tmp.y=y-v1.y;
        tmp.y=z-v1.z;

There's an error here, but I'm not telling you what.

Oops! Thanks for pointing that one out!

Your class design is not very object oriented. A vector object should be able to calculate its own length, there doesn't have to be a function to do that for them, if you know what I mean

Originally Posted by pronecracker

Your class design is not very object oriented. A vector object should be able to calculate its own length, there doesn't have to be a function to do that for them, if you know what I mean

This is a common fallacy.

"Object oriented" is not the same thing as "all functions to calculate some property of an object must be member functions of the class".

For discussion of cases, in C++, where implementing operations as member functions can actually reduce benefits associated with encapsulation of a class, have a look here.

So instead of v.VecLength(v) i should have v.VecLength?

No, not at all. What grumpy is saying is that just because a free function is used instead of a member function does not mean the code is any less object oriented.

Originally Posted by grumpy

This is a common fallacy.

"Object oriented" is not the same thing as "all functions to calculate some property of an object must be member functions of the class".

For discussion of cases, in C++, where implementing operations as member functions can actually reduce benefits associated with encapsulation of a class, have a look here.

eh?

I don't disagree that it's sometimes more practical to do things that way but it is not, by definition, OOP. it's unfortunate that C++ doesn't allow us to define non-member functions and yet have the option to call them as members, ie:

Code:

void
bar(foo const &);
 
foo foo;
bar(foo);
foo.bar();

that would allow you to do a lot of neat things, if you think about it.

Originally Posted by Sebastiani

I don't disagree that it's sometimes more practical to do things that way but it is not, by definition, OOP.

I'd be very interested to see an accepted definition of OOP that specifically requires that all operations on a class be members of a class.

Originally Posted by Sebastiani

it's unfortunate that C++ doesn't allow us to define non-member functions and yet have the option to call them as members, ie:

Code:

void
bar(foo const &);
 
foo foo;
bar(foo);
foo.bar();

that would allow you to do a lot of neat things, if you think about it.

If you really need to do that, provide both the member and non-member versions, and have one call the other.