CODEWITHSUNDEEP
c++arrayspointers
xadeka
xadeka

Reputation: 149

C++: The value that the pointer is pointing to changes

I am trying to code a class that represents a set of integers. It's a homework assignment but for the life of me I cannot figure out this issue.

In the class "IntSet", I have two private variables; one is a pointer to an array the other is the size of the array. I can create objects of this class and they work as intended. But I have this function named "join" that returns an object of the IntSet class. It essentially concatenates the arrays together then uses that array to create the returning object.

Here is my code:

#include <iostream>
using namespace std;

class IntSet {
        int * arrPtr;
        int arrSize;
    public:
        //Default Constructor
        IntSet() {
            int arr[0];
            arrPtr = arr;
            arrSize = 0;
        }
        //Overloaded Constructor
        IntSet(int arr[], int size) {
            arrPtr = arr;
            arrSize = size;
        }

        //Copy Constructor
        IntSet(const IntSet &i) {
            arrPtr = i.arrPtr;
            arrSize = i.arrSize;
        }

        /*
         * Returns a pointer to the first
         * element in the array
         */
        int* getArr() {
            return arrPtr;
        }

        int getSize() {
            return arrSize;
        }

        IntSet join(IntSet &setAdd) {

            //Make a new array
            int temp[arrSize + setAdd.getSize()];

            //Add the the values from the current instance's array pointer
            //to the beginning of the temp array
            for (int i = 0; i < arrSize; i++) {
                temp[i] = *(arrPtr + i);
            }

            //Add the values from the passed in object's array pointer
            //to the temp array but after the previously added values
            for (int i = 0; i < setAdd.getSize(); i++) {
                temp[i + arrSize] = *(setAdd.getArr() + i);
            }

            //Create a new instance that takes the temp array pointer and the
            //size of the temp array
            IntSet i(temp, arrSize + setAdd.getSize());

            //Showing that the instance before it passes works as expected
            cout << "In join function:" << endl;
            for (int j = 0; j < i.getSize(); j++) {
                cout << *(i.getArr() + j) << endl;
            }

            //Return the object
            return i;
        }

};

int main() {

    //Make two arrays
    int arr1[2] = {2 ,4};
    int arr2[3] = {5, 2, 7};

    //Make two objects normally
    IntSet i(arr1, 2);
    IntSet j(arr2, 3);


    //This object has an "array" that has arr1 and arr2 concatenated, essentially
    //I use the copy constructor here but the issue still occurs if I instead use
    //Inset k = i.join(j);
    IntSet k(i.join(j));

    //Shows the error. It is not the same values as it was before it was returned
    cout << "In main function:" << endl;

    for (int l = 0; l < k.getSize(); l++) {
        cout << *(k.getArr() + l) << endl;
    }

    return 0;
}

The program compiles and the output as of now is:

In join function:
2
4
5
2
7
In main function:
10
0
-2020743083
32737
-2017308032

I don't know why but the 10 and 0 are always the same every time I recompile and run. Also, if I print out the address of the pointer rather than the value(in both the join function and the main function), I get the same memory address.

Sorry if I misuse terms, I come from a java background, so pointers and such are a little new to me. If any clarification is needed, please ask.

Thanks in advance.

Upvotes: 2

Views: 238

Answers (2)

user3072164
user3072164

Reputation: 

int temp[arrSize + setAdd.getSize()];

This is a local array, its lifetime ends once the function returned.

IntSet i(temp, arrSize + setAdd.getSize());

Here you are constructing an IntSet with this array. In fact the constructor simply changes a member pointer to the value of temp:

IntSet(int arr[], int size) {
    arrPtr = arr;
    arrSize = size;
}

As a result, since the lifetime of the object that temp and consequently also i.arrPtr is pointing to ends after leaving join, you will have a wild pointer. Dereferencing this pointer later in main invokes undefined behavior.

You need to allocate the array dynamically with new[] and delete it later with delete[]. The same goes for your constructors. Also note that if you use new[] in join and delete[] in the destructor, then you also have to make sure that the copy constructor actually copies the array (create new array with new[] and copy contents). If you simply assign the pointer then both the source and destination object will point to the same array and they will also both try to delete it at deconstruction, again invoking undefined behaviour.

But since this C++, you might as well use a std::vector which does all of this for you. (or std::set if you actually want a integer set)

Upvotes: 4

Willem
Willem

Reputation: 927

The quickest fix with your code is to change

int temp[arrSize + setAdd.getSize()];

into this

int * temp = new int[arrSize + setAdd.getSize()];

The thing is that you allocated temp on the stack, so when join() returns that memory is releases. By allocating memory on the heap (as per the fix) the memory is not released when join() returns.

There are other things wrong with your code -- depending on the point of the assignment. I think most of these will be fixed when you consider the implications of having memory on the heap.

Upvotes: 0

Related Questions