Array Index Overload Bug

Question

Why does this code result in the second element of the array being printed as 0, irrespective of the value that was specified in the definition of the integer array object? The output of the code below is 7 0 3 4 5 6 instead of 7 2 3 4 5 6, what would be the reason for this behavior?

// Overloading operators for Array class
#include<iostream>
#include<cstdlib>

using namespace std;

// A class to represent an integer array
class Array
{
private:
    int *ptr;
    int size;
public:
    Array(int *, int);

    // Overloading [] operator to access elements in array style
    int &operator[] (int);

    // Utility function to print contents
    void print() const;
};

// Implementation of [] operator.  This function must return a
// reference as array element can be put on left side
int &Array::operator[](int index)
{
    if (index >= size)
    {
        cout << "Array index out of bound, exiting";
        exit(0);
    }
    return ptr[index];
}

// constructor for array class
Array::Array(int *p = NULL, int s = 0)
{
    size = s;
    ptr = NULL;
    if (s != 0)
    {
        ptr = new int[s];
        for (int i = 0; i < s; i++)
            ptr[i] = p[i];
        delete ptr;
    }
}

void Array::print() const
{
    for(int i = 0; i < size; i++)
        cout<<ptr[i]<<" ";
    cout<<endl;
}

// Driver program to test above methods
int main()
{
    int a[] = {1, 2, 3, 4, 5, 6};
    Array arr1(a, 6);
    arr1[0] = 7;
    arr1.print();
    arr1[8] = 6;
    return 0;
}

Answer 1

In the Array constructor, immediately after allocating and filling the dynamically allocated buffer at ptr, the buffer is released with

delete ptr;

All accesses of the buffer at ptr after this point invoke undefined behaviour. Side note: This should have been delete[] ptr; to ensure that the array was released correctly.

The solution: Don't do that!

Add a destructor to free ptr when Array goes out of scope and is done with the buffer.

// destructor for array class
Array::~Array()
{
    delete[] ptr;
}

The compiler will automatically generate a destructor for you, but that generic destructor is not qualified to know whether or not it is safe to delete[] what's at a pointer member. It might not be an array, the allocation could be owned by another object (See What is ownership of resources or pointers?) or perhaps not allocated dynamically with new.

This brings up a side note: The default special member functions that handle copying this object will mindlessly copy the pointer, not the allocation, and leave you with two objects pointing to the same allocation. Sooner or later this will be fatal because one copy will go out of scope before the other and if nothing else tries to access the freed allocation and break the program, the second delete[] will break the program. This issue and its solution is covered in detail at What is The Rule of Three?

The general rule of thumb is to not make a class like this and instead use std::vector. std::vector does all of this and a whole lot more.

Answer 2

I modified the code as such to include an explicit default constructor and copy constructor, also included the std::out_of_range exception but am not sure if the latter is properly implemented. This was done as an exercise in handling arrays without utilizing the vector container from STL. Added a swap member function and assignment operator but getting a few error msgs.

class "Array" has no member "swap" member "Array::size" (declared at line 12) is inaccessible 'operator=' must be a member function 'this' may only be used inside a nonstatic member function

// Overloading operators for Array class
#include<iostream>
#include<cstdlib>
//#include<vector>

using namespace std;

// A class to represent an integer array
class Array{
private:
    int *ptr;
    int size;
public:
    Array(int *, int);
    Array(const Array&);
    ~Array();
    Array& operator= (Array);

    // Overloading [] operator to access elements in array style
    int &operator[] (int);

    // Utility function to print contents
    void print() const;

    friend void swap(Array& first, Array& second);};


// Implementation of [] operator.  This function must return a
// reference as array element can be put on left side
int &Array::operator[](int index){
    // try {
    //     return ptr[index];}
    // catch(const out_of_range& oor){
    //         cerr << "Out of Range error: " << oor.what() << '\n';}    
    if (index >= size || index < 0){
       throw out_of_range("Index out of Range error");
    }
    return ptr[index];
}

// constructor for array class
Array::Array(int *p = NULL, int s = 0){
    size = s;
    ptr = NULL;
    if (s != 0){
        ptr = new int[s];
        for (int i = 0; i < s; i++)
            ptr[i] = p[i];}
}

// destructor for array class
Array::~Array(){
    delete[] ptr;
    ptr = NULL;}

// copy constructor for array class
Array::Array(const Array& A) { 
    size = A.size;
    ptr  = new int[size];
    for (int i = 0; i < size; i++)
        ptr[i] = A.ptr[i];}

void Array::swap(Array& first, Array& second){
    using std::swap;
    swap(first.size, second.size);
    swap(first.ptr, second.ptr);}

//Assignment operator for array class
Array::Array& operator=(Array other){
    swap(*this, other); 
    return *this;}

//print function for array elements
void Array::print() const{
    cout << "{";
    for(int i = 0; i < size; i++)
        cout<<ptr[i]<<" ";
    cout<<"}"<<endl;}

// Driver program to test above methods
int main()
{
    int a[] = {1, 2, 3, 4, 5, 6};
    Array arr1(a, 6);
    arr1[0] = 7;
    arr1.print();
    Array arr2 = arr1;
    arr2.print();
    arr1[-1] = 4;
    return 0;
} 

Answer 3

Modified print function to exclude space after last array element. Modified constructor method declaration to include initialized arguments. Added additional const version of index[ ] operator overload but don't think it's properly implemented or if it would actually be utilized.

#include<iostream>
#include<cstdlib>


// A class to represent an integer array
class Array{
private:
    int *ptr;
    std::size_t size;
public:
    Array(int *p = nullptr, std::size_t s = 0);
    Array(const Array&);
    ~Array();
    Array& operator= (Array);

    // Overloading [] operator to access elements in array style
    int &operator[] (std::size_t);

    int const& operator[](std::size_t) const;

    // Utility function to print contents
    void print() const;

    friend void swap(Array& first, Array& second);};


// Implementation of [] operator.  This function must return a
// reference as array element can be put on left side
int &Array::operator[](std::size_t index){
    puts("overload");   
    if (index >= size || index < 0){
       throw std::out_of_range("Index out of Range error");
    }
    return ptr[index];
}

int const& Array::operator[](std::size_t index) const{
    puts("const overload");
    if (index >= size || index < 0){
        throw std::out_of_range("Index out of Range error");
    }
    return ptr[index];
}


// constructor for array class
Array::Array(int *p, std::size_t s){
    size = s;
    ptr = nullptr;
    if (s != 0){
        ptr = new int[s];
        for (int i = 0; i < s; i++){
            ptr[i] = p[i];}
    }
}

// destructor for array class
Array::~Array(){
    delete[] ptr;
    ptr = nullptr;}

// copy constructor for array class
Array::Array(const Array& A) { 
    size = A.size;
    ptr  = new int[size];
    for (int i = 0; i < size; i++){
        ptr[i] = A.ptr[i];}
}
//swap friend function of assignment operator
void swap(Array& first, Array& second){
    using std::swap;
    swap(first.size, second.size);
    swap(first.ptr, second.ptr);}

//Assignment operator for array class
Array& Array::operator=(Array other){
    swap(*this, other); 
    return *this;}

//print function for array elements
void Array::print() const{
    std::cout << "{";
    for(int i = 0; i < size; i++){
        std::cout << ptr[i];
        if (i == size-1){
            continue;}
        std::cout<<" ";       
       }
    std::cout<<"}"<< std::endl;}

// Driver program to test above methods
int main()
{
    int a[] = {1, 2, 3, 4, 5, 6};
    Array arr1(a, 6);
    std::cout << arr1[3] << '\n';
    arr1[4] = 7;
    arr1.print();
    Array arr2 = arr1;
    arr2.print();
    arr1[-1] = 4;
    return 0;
}