Can't assign array element to class variable while overloading "=" operator

Question

I am quite new in C++ programming, so maybe that's why I can't figure out why this assignment is not working. In my class, I want to overload "=" operator. I have specified a function, that outputs variables as an array. In overloading, I want to assign those variables to new object.

 obj_new = obj_with_variables

overloading:

  obj_new_x=obj_with_values_parameters()[0];
  obj_new_y=obj_with_values_parameters()[1];

Here is a code:

// Test1.cpp : Defines the entry point for the console application.
//
#include "stdafx.h"
#include <iostream>
#include "string"

using namespace std;

class Vector2D
{
public:
Vector2D()
{

}
Vector2D(int& a, int& b)
    :x(a), y(b)
{

}
void Drukuj()
{
    cout << "wektor [" << x << ',' << y << "] \n";
}
void Wspolrzedne(int&a, int&b)
{
    x = a;
    y = b;
}
int* Wspolrzedne()
{
    int tab[2] = { x,y };
    return tab;
}
void operator = (Vector2D& obj)
{
    int* a = obj.Wspolrzedne();
    cout << a[0] << "\n";
    x = a[0];
    cout << x << " what? \n";
    y = a[1];
}

private:
int x, y;

};


int main()
{
int x1 = 2, x2 = 3;
Vector2D wektor(x1, x2);
wektor.Drukuj();
Vector2D wektor2;
wektor2 = wektor;
wektor2.Drukuj();
wektor.Drukuj();


return 0;
}

The problem is that it assigns some strange values. However, if I don't use a reference, but declare 2 int values (j,k) and assign array element to them [0,1], it works fine. Also, when using static numbers (for example, instead of a[0] ; use "2") it works fine too.
What is going on?
I would be glad if somebody could point me to right answer/ resources.
Regards,

Answer 1

Your code has undefined behavior because operator= is accessing invalid data. Wspolrzedne() is returning a pointer to a local variable tab that goes out of scope when Wspolrzedne() exits, thus the a pointer used in operator= is not pointing at valid data.

If you want Wspolrzedne() to return multiple values, you need to have it return (by value) an instance of a struct or class to hold them. Try something more like this:

#include "stdafx.h"
#include <iostream>
#include <string>

using namespace std;

struct Coordinate {
    int x;
    int y;
};

class Vector2D
{
public:
    Vector2D()
        : x(0), y(0)
    {
    }

    Vector2D(int a, int b)
        : x(a), y(b)
    {
    }

    Vector2D(const Coordinate &c)
        : x(c.x), y(c.y)
    {
    }

    Vector2D(const Vector2D &src)
        : x(src.x), y(src.y)
    {
    }

    void Drukuj()
    {
        cout << "wektor [" << x << ',' << y << "] \n";
    }

    void Wspolrzedne(int a, int b)
    {
        x = a;
        y = b;
    }

    void Wspolrzedne(const Coordinate &c)
    {
        x = c.x;
        y = c.y;
    }

    Coordinate Wspolrzedne()
    {
        Coordinate c = { x, y };
        return c;
    }

    Vector2D& operator=(const Vector2D &obj)
    {
        Coordinate c = obj.Wspolrzedne();
        cout << c.x << ',' << c.y << "\n";
        Wspolrzedne(c);
        return *this;
    }

private:
    int x;
    int y;
};

On the other hand, there is no real reason to even have operator= call Wspolrzedne() to get the coordinates at all, when it can just access obj.x and obj.y directly instead:

Vector2D& operator=(const Vector2D &obj)
{
    x = obj.x;
    y = obj.y;
    return *this;
}

In which case, you can simply eliminate your operator= altogether, and let the compiler provide a default-generated implementation that does the exact same thing for you.

Answer 2

Your interface is dangerous, as returning raw pointers from functions means the caller must know how to manage it. That is, the caller needs to be astutely aware of the answers to questions like, "should the caller delete the pointer or does the object I got it from retain ownership?" What is this pointer pointing to? If it's an array, now many elements are there? If I must delete it, do I use delete or delete[]?

And so on. This is not good because it's very, very error prone.

Next you have a function that returns a pointer to stack-local data. When the function returns, that memory is already invalid so the return value is always going to result in undefined behavior to read.

int* Wspolrzedne()
{
    int tab[2] = { x,y };    
    return tab;  // BAD - returns pointer to function stack data
}

Where does tab live? It's destroyed when the function exits, but you're returning a pointer to it.

This is a dangerous interface and should be changed. Perhaps you should return the values as a pair:

std::pair<int, int> Wspolrzedne()
{
    return std::pair{ x,y }; // assuming c++17
    // return std::pair<int, int>{x, y}; // older compilers
}

An interface like this avoids pointers, and removes all the issues mentioned above.

Answer 3

In your member function int* Wspolrzedne(), you return the address of local variable tab. Accessing this variable once its life time has ended, as you do in your = operator, is undefined behaviour.