0

I know this question has appeared lots of times here and on the internet, but even by searching on this site I can't overload the + operator in myvec class. The strange thing is that I can overload the = operator, but as I write the declaration and definition of the operator +, I get an error.

More specifically, I declare the operator as

myvec& myvec::operator+(const myvec& v, const myvec& w)

and the definition is

myvec& myvec::operator +(const myvec& v, const myvec& w)
{
    int d = v.size();
    myvec x(d);
    for (int i = 0; i < d; i++) {
        x(i) = v(i)+w(i);
    }
    return x;
}

In the following my little class:

#include <iostream>
#include <cmath>

using namespace std;

class myvec {
private:
    int dimension;
    double* data;

public:
    myvec(int dim);
    myvec(const myvec& v);
    ~myvec();

    int size() const;
    void Print();
    double& operator ()(int i);
    myvec& operator =(const myvec& v);
    myvec& operator +(const myvec& v, const myvec& w);
};


myvec::myvec(int dim)
{
    dimension = dim;
    data = new double[dim];

    for (int i = 0; i < dimension; i++) {
        data[i] = 0.0;
    }
}

myvec::myvec(const myvec& v)
{
    int dimensione = v.size();
    data = new double[dimensione];
    for (int i = 0; i < dimensione; i++) {
        data[i] = v.data[i];
    }
}

myvec::~myvec()
{
    dimension = 0;
    delete[] data;
    data = NULL;
}

int myvec::size() const
{
    return dimension;
}

double& myvec::operator ()(int i)
{
    return data[i];
}

myvec& myvec::operator =(const myvec& v)
{

    int dim  = v.size();
    for (int i = 0; i < dim; ++i) {
        data[i] = v.data[i];
    }
    return *this;
}

myvec& myvec::operator +(const myvec& v, const myvec& w)
{
    int d = v.size();
    myvec x(d);
    for (int i = 0; i < d; i++) {
        x(i) = v(i)+w(i);
    }
    return x;
}

void myvec::Print()
{
    for (int i = 0; i < dimension; i++) {
        cout << data[i]<<endl;
    }
}

The compiler gives me the error:

testmyvec.cpp.cc:77:59: error: ‘myvec& myvec::operator+(const myvec&, const myvec&)’ must take either zero or one argument

It's clearly referring to the definition of the + operator. How can I fix it in order to overload the operator?

Jorengarenar
  • 2,705
  • 5
  • 23
  • 60
FEGuy
  • 57
  • 8
  • Is there a special reason you chose to overload `()` instead of the more common `[]`? – Waqar Jul 07 '20 at 14:11
  • no, just because my book uses `()`. But that's working fine in my code. The problem is with `+` @Waqar – FEGuy Jul 07 '20 at 14:11
  • 2
    Remove the `&` from the return type, or there will be undfined behaviour. – molbdnilo Jul 07 '20 at 14:11
  • Change the book. It must be really terrible if it is teaching you such things – Waqar Jul 07 '20 at 14:12
  • @molbdnilo This was the declaration given in my book. It should be fine I guess – FEGuy Jul 07 '20 at 14:13
  • @Waqar Thanks for the advice :) Btw, could you tell me how to fix this in order to let the compiler compile? Even if I remove `&`, it's still not working – FEGuy Jul 07 '20 at 14:14
  • @VoB `myvec& myvec::operator=(const myvec& v)` -- Your assignment operator leaks memory. Your `vec` class has incorrect copy semantics because of this. Thus any talk of `operator +` is a moot point until you fix the copying issues. The second thing is that `operator +` should be returning a brand new `vec`, not a reference to the original vector. Returning a reference to the original vector should be the job of `operator +=`, not `operator +`. – PaulMcKenzie Jul 07 '20 at 14:16
  • 1
    @VoB Then your book is wrong. – molbdnilo Jul 07 '20 at 14:17
  • @VoB Just for the record, what book are you learning this from? There are multiple things wrong with what you've posted, both in design and bugs. – PaulMcKenzie Jul 07 '20 at 14:21
  • Does this answer your question? [What are the basic rules and idioms for operator overloading?](https://stackoverflow.com/questions/4421706/what-are-the-basic-rules-and-idioms-for-operator-overloading) – Richard Critten Jul 07 '20 at 14:21
  • Thanks for all the multiple corrections. The book is "Parallel Scientific Computing in C++ and MPI A Seamless Approach to Parallel Algorithms and their Implementation" – FEGuy Jul 07 '20 at 14:26
  • I think it's a case of the author knowing their discipline, but when applying C++ to implement it, well, that's another story. – PaulMcKenzie Jul 07 '20 at 14:29
  • 1
    @VoB -- [See this to fix your assignment operator](http://coliru.stacked-crooked.com/a/61a2f46730148742) – PaulMcKenzie Jul 07 '20 at 14:38
  • Thanks @PaulMcKenzie for this last corrections ! Just one last question/check: The syntax `myVector& myVector::operator+(const myVector &v, const myVector &wj)` is *wrong* because it returns a reference to a local object, while I should return a new myvec object, right? – FEGuy Jul 07 '20 at 14:52
  • yes, you should be returning a new object, not a reference. – PaulMcKenzie Jul 07 '20 at 14:58
  • Perfect, thanks again! :) – FEGuy Jul 07 '20 at 15:08

2 Answers2

2

Complier already gave you the answer: "must take either zero or one argument".

Declaration should be myvec operator+(const myvec&); and in definition you use this like so:

myvec myvec::operator +(const myvec& w)
{
    int d = v.size();
    myvec x(d);
    for (int i = 0; i < d; i++) {
        x(i) = (*this)(i) + w(i);
    }
    return x;
}

And don't return reference to local objects.

Jorengarenar
  • 2,705
  • 5
  • 23
  • 60
1

Move the overloaded operator out of the myvec class. Also don't return references to local variables, change the return type to myvec.

class myvec{
    ...
};

myvec operator+(const myvec& v, const myvec& w);

myvec operator+(const myvec& v, const myvec& w) {
    int d = v.size();
    myvec x(d);
    for(int i=0;i<d;i++) {
        x(i)=v(i)+w(i);
    }
    return x;
}

operator+ takes two arguments, if you declare it inside a class and you give it two regular arguments then you've ended up with an operator+ that is taking three arguments, which isn't allowed.

For comprehensive advice look here

john
  • 85,011
  • 4
  • 57
  • 81
  • Thanks. I did exactly what you wrote, but it does not compile. It says: `no match for call to ‘(const myvec) (int&)’ – FEGuy Jul 07 '20 at 14:23
  • The other issue is that `operator=` is broken in that it leaks memory. Thus copying will also be broken. – PaulMcKenzie Jul 07 '20 at 14:24
  • The line with `x(i) = v(i)+w(i)` @john – FEGuy Jul 07 '20 at 14:25
  • @VoB OK as other have said you have multiple errors in your code. This one can easily be fixed, you need to add a const version of your `operator()`. – john Jul 07 '20 at 14:26
  • @VoB `class myvec { ... double operator()(int i) const; ... };` – john Jul 07 '20 at 14:27
  • inside operator+ `v` and `w` are (correctly) const. But you don't have a const version of your `operator()`. That's what the compiler is complaining about. – john Jul 07 '20 at 14:28
  • Now it workds, but why do I have to add a const version? @john – FEGuy Jul 07 '20 at 14:30
  • If you don't have a const version you can't call operator() on a const myvec, which would obviously be a serious limitation. Every method you write should be marked const if it doesn't change the object, or cannot be used to change the object, like your size method for instance. Sometimes this means you need const and non-const versions of the same method. – john Jul 07 '20 at 14:34