My Stats class prints garbage

Question

Consider the following class:

class Stats
{
    private:
    int arraySize; // size of array

    int * data; // pointer to data, an array of integers

    // default size of array in default constructor
    static const int DEFAULT_SIZE = 10;

    public:
    Stats() // default constructor
    {
        arraySize = DEFAULT_SIZE; // element count set to 10

        data = new int[DEFAULT_SIZE]; // array of integers
        srand((unsigned int) time(NULL)); // seeds rand() function

        // initializes integer array data with random values
        for (int i = 0; i < DEFAULT_SIZE; i++)
        {
            // data filled with value between 0 and 10
            data[i] = rand() % (DEFAULT_SIZE + 1);
        }
    }

    ~Stats() // destructor that deletes data memory allocation
    {
        delete [] data;
    }

    void displaySampleSet(int numbersPerLine)
    {
        cout << "Array contents:" << endl; // user legibility

        // iterates through array and prints values in array data
        for (int i = 0; i < arraySize; i++)
        {
            cout << data[i];

            /*  nested if statements that either prints a comma between
            values or skips to next line depending on value of numbersPerLine   */
            if (i + 1 < arraySize)
            {
                if ((i + 1) % numbersPerLine != 0)
                    cout << ", ";
                else
                    cout << endl;
            }
        }
    }
}

For some reason, when I create a Stats object in the following way:

Stats statObject = Stats();

and then call the displaySampleSet() on it, the numbers display fine. However, the function prints garbage when the Stats object is created in the following way:

Stats statObject;
statObject = Stats();

I have no idea why it does this and have a feeling it has to do with the integer pointer 'data' and/or with the way the object is being created but I'm not sure what... Any and all help is fully appreciated! Thank you so much in advance.

Update: Destructor added

You need to implement `operator=` and `copy constructor` for this class. — Kiril Kirov, Apr 14 '13 at 11:01

score 2 · Accepted Answer · edited May 23 '17 at 12:12

2

Both the code statements produce undefined behavior. It is lucky that the first one works while second doesn't.

Stats statObject = Stats();

Is copy initialization. It copies the created Stats object by calling the default constructor and then copies it in to statObject by calling the copy constructor. Note your class does not provide an copy constructor so the implicitly generated one gets used. This creates a shallow copy of the dynamically allocated members. The object from which this copy gets created is eventually destroyed and then whats left in your class is dangling pointers which do not point to anything valid. The copy constructor needs to do a deep copy.

Same is the case with:

Stats statObject;
statObject = Stats();

Wherein the compiler generated copy assignment operator is called. Resulting in similar problems as in case 1.

You need to follow the Rule of Three and provide your copy constructor and copy assignment operators for the class.

edited May 23 '17 at 12:12

Community

1
1

answered Apr 14 '13 at 11:06

Alok Save

202,538
53
430
533

This is the default you haven't implemented everything answer and I thought this was the case too, but they haven't defined a destructor and, hence, won't release the memory... – Alex Chamberlain Apr 14 '13 at 11:07
My textbook mentions the copy constructor but does not mention anything about a "copy assignment operator". I'm not sure what that means... – user1800967 Apr 14 '13 at 11:12
@user1800967 It would have the following prototype... `Stats& operator=(Stats const& rhs)` or `Stats& operator(Stats rhs)` depending on your mood. – Alex Chamberlain Apr 14 '13 at 11:15
@AlexChamberlain: I believe it was a typo that OP missed the destructor in a copy paste. – Alok Save Apr 14 '13 at 11:19
@Alok Save actually, I was copying and pasting from a project I am working on currently and I'm trying to minimize how much code I'm pasting. – user1800967 Apr 14 '13 at 11:20

score 1 · Answer 2 · answered Apr 14 '13 at 11:14

You code is using synthesized copy constructor , assignment operator defined by compiler. Also you are not freeing the memory allocated dynamically for default constructor. you have to define your own copy construcor, overload assignment operator( follow rule of three).

Both the cases are undefined behaviour

Stats statObject;
statObject = Stats();  //using compiler defined assignment operator.

Stats statObject = Stats();  //using compiler defined copy constructor.

Here temp object generated is is using assignment operator overloaded by compiler and is doing shallow copy instead of deep copy. overload assignment operator:

Stats& operator=(const Stats& rhs);

define copy constructor:

Stats(const Stats& rhs);

My Stats class prints garbage

2 Answers2