Error's on overloaded operators = and +

[grenoble12]

Hi there,
I'm trying to build a class that holds 2d coordinates, and as far as i can see, i've done everything i ought to in order to define the class. When i compile the code though, it gives me a few different error methods.

Every time i use "this" i get an error message:

error: invalid use of 'this' in non-member function

and for the '=' operator, i get two other error messages:

error: 'two_vector& operator=(two_vector&)' must be a nonstatic member function
error: 'two_vector& operator=(two_vector&)' must take exactly two arguments

I've searched around, but all i can find on the topic has proved hard to relate back to my code. More annoyingly, i tried defining the methods inside the class definition (replacing the prototypes) and in that case, the code compiles, and functions fine. This is my code:

class two_vector{
	int x, y;
public: 
	int get_x() { return x; }
	int get_y() { return y; }
	void set_x(int a) { x = a;}
	void set_y(int a) { y = a;}
	two_vector();
	two_vector(int, int);
	two_vector operator+ (two_vector&);
	two_vector& operator= (two_vector&);
};

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

two_vector::two_vector(int a, int b) {
	x = a;
	y = b;
}

two_vector::two_vector operator+ (two_vector& rhs){
	int a = rhs.get_x() + this->get_x();
	int b = rhs.get_y() + this->get_y();
	return two_vector(a,b);
}

two_vector::two_vector& operator= (two_vector& rhs){
	if (&rhs != this){
		this->set_x(rhs.get_x());
		this-> set_y(rhs.get_y());
	}
	return *this;	
}

I'd be greatly if anyone can help. Really sorry if this is a blindingly obvious mistake on my part..

Many thanks

[Galik]

You got your qualifiers a bit mixed up:

two_vector two_vector::operator+ (two_vector& rhs){
	int a = rhs.get_x() + this->get_x();
	int b = rhs.get_y() + this->get_y();
	return two_vector(a,b);
}

two_vector& two_vector::operator= (two_vector& rhs){
	if (&rhs != this){
		this->set_x(rhs.get_x());
		this-> set_y(rhs.get_y());
	}
	return *this;
}

[grenoble12]

Ah..*bows head*

Well i guess on the bright side it was easy to resolve.. Thanks a lot Galik!

[Disch]

also note you should be const correct:

//  two_vector operator+ (two_vector&);  //  bad
two_vector operator + (const two_vector&) const; // good

//  two_vector& operator= (two_vector&);  // bad
two_vector& operator = (const two_vector&); // good 

[grenoble12]

Disch,
I made the changes you suggest, but it now rather sadistically gives me six more errors:

error: passing 'const two_vector' as 'this' argument of 'int two_vector::get_x()' discards qualifiers
error: passing 'const two_vector' as 'this' argument of 'int two_vector::get_y()' discards qualifiers

So i fear you might have increased your workload but can you help me out with this? I tried adding const to quantify get_x and get_y, but that made things worse.. In order to see if it was all get_x and get_y's I redefined the method to be:

two_vector two_vector::operator+ (const two_vector& rhs) const{
	int a = rhs.get_x();
	int b = this->get_x();
	int c = rhs.get_y();
	int d = this->get_y();
	return two_vector(a+b,c+d);
}

Which only showed that it really is every get_x and get_y.. Im nervous this is once again something slightly obvious.. i dont suppose you have a newbie point system at all?

Also, for the + operator, whats the purpose of the const at the end of the prototype?
Thanks a lot

[Disch]

well for starters... do you really need get/set fucntions for x and y or are those useless getters/setters?

My guess is they're useless, so I would get rid of them and just make x and y public. There's nothing wrong with public members where appropriate. Useless getters/setters are a waste of time and energy and make the interface weird.

But if they're useful, the solution is to make get_x and get_y const:

//int two_vector::get_x();  // bad
int two_vector::get_x() const;  // good

// same for get_y() 

Also, for the + operator, whats the purpose of the const at the end of the prototype?

It makes the function const.

const functions "promise" not to change the state of the object. Therefore they can be called by const objects.

non-const functions might change the state of the object, therefore they can only be called by nonconst objects.

class example
{
public:
  void nonconst() { }  // a non-const function
  void yesconst() const { }  // a const function
};

int main()
{
  example a;
  a.nonconst();  // OK
  a.yesconst();  // OK

  const example b;
  b.nonconst();  // ERROR, const object calling nonconst function
  b.yesconst();  // OK
}

Similarly, const functions can only call other const functions. This is the error you were getting. Your + operator (const) was trying to call get_x (nonconst). Since get_x might change the state of the object, this breaks the "promise", and therefore the compiler barks at you.


EDIT:

I just realized you posted the entire class above!

Yes, get_x/get_y/set_x/set_y are useless getters/setters. Get rid of them. x and y should be public for this class.

[grenoble12]

Right then, i'll set about removing the setters and getters. What situations are setters and getters useful?

To clarify though, to declare a function as const, the const goes after the name of the function, but before the {}'s ? Why is it different to when a const variable is declared, or can this also be done after the variable name?

Cheers

[sohguanh]

To clarify though, to declare a function as const, the const goes after the name of the function, but before the {}'s ? Why is it different to when a const variable is declared, or can this also be done after the variable name?

You are looking for some consistent naming convention is it ? Unfortunately, C++ works in a myriad ways.

int var const = 0; // compile error
const int var = 0; // compile ok

This is something Bjarne Stroustrop C++ creator should have know when he create the language! :P

[Galik]

grenoble12 wrote:

To clarify though, to declare a function as const, the const goes after the name of the function, but before the {}'s ? Why is it different to when a const variable is declared, or can this also be done after the variable name?

When you access a variable you tell the compiler if the returned value is const or not:

const int value; // returns a const 

When you access a function you tell the compiler if the returned value is const or not:

const int object.function(); returns a const, non const function

So we need a different way to tell the compiler that the function itself will not modify its object. So we put const after the function parameters to signify this:

const int object.function() const; returns a const, const function

[jsmith]

What situations are setters and getters useful?

When the class needs to enforce some invariant. For example, if your two_vector class only allowed
x and y to be even numbers, you could:

  bool set_x( int new_x ) {
      if( new_x % 2 )
          return false;
      x = new_x;
      return true;
  }

Or, if you need to provide synchronization (ie, the invariant of the class is thread safety), then a setter and
getter would acquire the necessary locks, perform the read or write operation, and free the locks.

Otherwise, setters and getters are simply red tape with no real benefit. Setters and getters generally break
encapsulation since they imply that the user of the object knows better what the values should be than the
object itself; the object is simply a container of those values.

EDIT: Note that if this is for homework, your professor is probably looking for a "purer" solution which would
mean writing the useless setters and getters anyway, even though professionally I would eschew them in
favor of direct access.

[grenoble12]

Thanks peeps, Galik that makes a lot of sense and jsmith this isnt for homework, ive just found myself with some free time and decided to remind myself of C++. I remember my teachers telling us to include them on some pretty similar class to this one, which is why i though they were necessary or at least safer to include..
Thanks again for all the advice.