0

I have read some article on Stackoverlow like How to “return an object” in C++? about this problem but my problem still exists.

My program implements a Vector Class (vector in Maths). I want to overload an operator +, I have try to modify empty object in function or use static member and the results is in my comments on these code below.

This is my code:

#include <iostream>

class Vector {
    double * arr;
    unsigned short dim;

public:
    Vector(const int& d = 0); // This constructor initialize memory for its member

    Vector& operator = (const Vector& v);

    /* This function must return a Vector object
     *
     * Example: Vector v = v1 + v2; // v1.arr = {1, 1, 3}, v2.arr = {1, 0, 1}
     *
     * Then v.arr = {2, 1, 4} // this is just an example

     * When I run it by using empty class like:
     * Vector operator + (const Vector& v) const {
     *     Vector vec();
     *     // Do something with vec;
     *     return vec;
     * }
     * It returns a garbage value.
     *
     * When I run the code below, the GNU compiler inform:
     * C:\Desktop>g++ -Wall -ansi -c Vector.cpp
     * C:\Desktop>g++ -Wall -ansi -o main.exe main.cpp vector.o
     * vector.o:Vector.cpp:(.text+0x33e): undefined reference to `Vector::_vec'
     */

    Vector operator + (const Vector& v) const {

        if (dim == v.dim) {
            _vec = Vector(dim);

            for (int i = 0; i < dim; ++i) {
                _vec.arr[i] = arr[i] + v.arr[i];
            }

            return _vec;
        }

        return Vector(0);
    } 

    ~Vector();

private:
    static Vector _vec;
};

Main function for anyone needs it:

#include <iostream>
#include "Vector.h"

using namespace std;

int main(int argc, char const *argv[]) {
    Vector v(-2), v3;
    Vector v2(2);

    cout << "Input vector: ";
    cin >> v;
    cout << v << endl;
    cout << "Input vector: ";
    cin >> v2;
    cout << v2 << endl;

    v3 = v + v2;
    cout << v3;

    return 0;
}

Thanks for reading.

Community
  • 1
  • 1
mja
  • 1,273
  • 1
  • 20
  • 22
  • Can you explain how this does not work? Compiler errors? Logic errors? – NathanOliver May 05 '16 at 12:45
  • _"It doesn't work"_ is pretty vague. Could you provide a [MCVE] please. – πάντα ῥεῖ May 05 '16 at 12:45
  • @πάνταῥεῖ Thanks, I will implement it. – mja May 05 '16 at 12:48
  • On '+' operator it is possible that the new Vector to be allocated on stack instead of heap and therefore when the '+' function exits to have references to objects that does not exists anymore. You should use new operator to create new Vector instances and then return references to newly created objects – Luci May 05 '16 at 12:59
  • Or you could add a copy constructor since the default copy constructor will copy the values as memory copy and therefore both instances will have the same the pointer to arr which is not OK – Luci May 05 '16 at 13:05

3 Answers3

2

I assume it's meant to be regular Vector of doubles (or more like array, since you don't change size yet). And operator+ is memberwise addition of two vectors.

There's lots of begginer mistakes in the sample, so let's get you up to speed first:

  1. Don't use other primitive types other than int, double, char or size_t without good reason (as you do with short).
  2. Don't mix signed and unsigned (as you do with dim and d) and don't mix types (as you do with i and dim) without good reason.
  3. Don't pass primitive type by const reference (as you do with const int& d)
  4. It's better to use constructor initializer list for simple things, instead of initializing in the body of constructor
  5. unsigned can't be smaller than 0, so the if (dim < 0) is redundant.
  6. You can pass 1 or even 0 to new[], so your if conditions are largely redundant
  7. You have to delete[] the memory you borrowed. The destructor would be a great place to do that.
  8. BTW you declared but not defined your destructor.
  9. The static Vector _vec; has absolutely no bussiness being there as far as I can see.
  10. Oh. Do not return through static vector. Create new one: Vector resultV(dim); and return that.

UPDATE1:

  1. The Vector(-2) will return something evil because you didn't follow 2. Most likely will return Vector(USHORT_MAX - 2).
  2. To get it form cin and print it from cout you need to overload istream& operator>>( istream&, Vector ) and ostream& operator<<( ostream&, Vector )

Apply those and if your problems persist, edit your question accordingly.

krOoze
  • 12,301
  • 1
  • 20
  • 34
  • I have learnt these rules but I don't realize these errors until you tell me. It's great. Thank you very much. – mja May 05 '16 at 13:47
1

For your immediate problem, you declare the static member but you don't allocate its memory anywhere.

You need to have

Vector Vecor::_vec;

in one of the .cpp files.

Also, relying on static variable to return result from an operation is a bad idea. Better look into move semantics.

1

I analyzed your coud and found following problems:

General errors:

  • You missed to declare a copy constructor.
  • You missed to declare a function for accessing vector values (operator[]).

Constructor Vector(const int& d = 0)

  • Passing an int by const reference is not wrong but useless.
  • The if (dim == 1) case is wrong: you can't use the same pointer variable to store a double and double[]

Method Vector& operator=(const Vector& v)

  • It don't handles the cases dim = 0 and dim = 1 (anyway the case dim = 1 is wrong also in constructor and don't really needs a separate handling).

Method Vector operator+(const Vector& v) const

  • No need to use a static variable here. Probably the error you get is a consequence of errors in constructor.

Destructor ~Vector();

  • Implementation is missing: it should free dynamic memory allocated by other functions

Here is the correct code:

#include <iostream>

class Vector 
{
    double* arr;
    int     dim;

public:

    Vector(int d = 0) 
    {
        if(dim < 0)
        {
            std::cout << "Dimension could not less than 0. It will be set to 0." << std::endl;
            // Better to throw an exception here!
        }

        if(dim <= 0) 
        {

            dim = 0;
            arr = NULL;
        }
        else
        {
            dim = d;
            arr = new double[dim];
        }
    } 

    const double& operator[](int i) const 
    {
        return arr[i]; 
    }

    double& operator[](int i) 
    {
        return arr[i]; 
    }

    Vector(const Vector& v)
    {
        dim = v.dim;

        if(dim > 0)
        {
            arr = new double[dim];

            for(int i = 0; i < dim; ++i) 
            {
                arr[i] = v.arr[i];
            }
        }
    }

    Vector& operator=(const Vector& v) 
    {
        if(this != &v) 
        {
            delete[] arr;

            dim = v.dim;

            if(dim > 0)
            {
                arr = new double[dim];

                for(int i = 0; i < dim; ++i) 
                {
                    arr[i] = v.arr[i];
                }
            }
        }

        return *this;
    }

    Vector operator+(const Vector& v) const 
    {
        if(dim == v.dim) 
        {
            Vector r(dim);

            for(int i = 0; i < dim; ++i) 
            {
                r.arr[i] = arr[i] + v.arr[i];
            }

            return r;
        }

        return Vector(0); // Better to throw an exception here!
    } 

    ~Vector()
    {
        if(arr != NULL) delete[] arr;
    }
};

void main()
{
    Vector v1(3);
    Vector v2(3);

    v1[0] = 1;
    v1[1] = 2;
    v1[2] = 3;

    v2[0] = 2;
    v2[1] = 4;
    v2[2] = 6;

    Vector v3 = v1 + v2;

    std::cout << std::endl << v3[0];
    std::cout << std::endl << v3[1];
    std::cout << std::endl << v3[2];
    std::cout << std::endl;
}

The main function produces the output:

3
6
9
Carlo
  • 1,539
  • 1
  • 11
  • 25