0
\$\begingroup\$

The code below is a custom array class that handles indexing, copying, printing, etc. explicitly.
Is there a better approach for specification (declaration and definition) of the assignment operator (with swap function) and the index [] operator?
Furthermore, are there any improvements that can be made to the other methods to improve functionality?

// 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 *p = NULL, int s = 0);
    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){   
    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, int s){
    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];}

//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{
    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;
} 
\$\endgroup\$
2
\$\begingroup\$

This isn't really a code review so much as help on how to get this to compile.

  • Array::Array(int *p = NULL, int s = 0){ default parameters go on the declaration, not the definition.*
  • Array::Array& operator=(Array other){ Qualify the operator with Array::, not the return type. You're not returning an Array::Array**, you're defining the operator= member function of Array
  • void Array::swap(Array& first, Array& second){ swap is not a member of array, it's a friend. Remove the Array::

*This is because the client (caller) to the method needs to know what the defaults are supposed to be, since default parameters are mostly a syntactic shortcut. If you put the defaults into the definition (e.g. a cpp/.o file), a client which includes the header cannot see the definition and thus cannot use them. In other words, there exists no Array::Array() constructor right now, there's only the Array::Array(int *, int), but through C++ magic it can look as though Array::Array() is being called.

** An Array::Array type is impossible because you can't name a child type the same as its parent type. It would be ambiguous with the constructor.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks for the feedback! Is it possible to avoid the Aborted (core dumped) message when the out_of_range error is thrown? \$\endgroup\$ – Darnoc Eloc Feb 12 at 23:37
  • \$\begingroup\$ Furthermore, how would one implement a try-catch statement for invalid index values? \$\endgroup\$ – Darnoc Eloc Feb 12 at 23:47
  • 1
    \$\begingroup\$ I would argue that you should not throw an exception on [], but that's debatable. My reasoning is that [] is canonically a dangerous operation in C++, and should be semantically thought of as a dereference of a potentially out-of-bounds address, usable only if you're absolutely certain it will not fail. This type of dangerous operation is integral to C++ since there's no convenient layer below it to handle it better. A method call like GetAt(IndexType) or something might throw by testing the bounds -- not by catching another exception, the class knows whether the argument is valid or not. \$\endgroup\$ – butt Feb 13 at 0:02
1
\$\begingroup\$

Don't using namespace std; - especially not in a header, where it inflicts the harm on every source that includes the header.

Prefer nullptr to NULL, because the former is more strongly typed.

Use std::size_t for indexing, rather than int.

When overloading operator[], it's usually necessary to provide two versions:

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

Usually, operator[] doesn't throw. Provide a separate "checked" interface if your clients need that as well. That's normally named at().

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.