Generic Array (Vector) Class C++

Question

I am a mathematician trying to learn how to become a better programmer in C++. I am currently now trying to write the main data structures from scratch. I have now completed a generic array class and I wanted to see if I was missing anything or if there is anything I can improve on.

Here is the header file:

#ifndef Array_h
#define Array_h

#include <iostream>

template <class T>
class Array {
private:
    int maxSize; // capacity of array
    int length;
    T *array;

public:
    Array(int size = 100);
    ~Array();
    void print() const;
    bool isEmpty() const;
    bool isFull() const;
    int maxListSize() const;
    int listSize() const;
    T front() const;
    T back() const;
    void swap(T& x, T& y);
    void sort();
    int search(const T& item);
    void insert(int location, const T& item);
    void insertAtTop(const T& item);
    void deleteItem(const T& item);

    void operator=(const T& temp);
    T& operator[](int index) { return array[index];}
};

template <class T>
Array<T>::Array(int size) {
    if (size <= 0) {
        std::cout << "Size cannot be less than 0";

        maxSize = 100;
        array = new T[maxSize];
    }
    else {
        maxSize = size;
        length = 0;
        array = new T[maxSize];
    }

}

template <class T>
Array<T>::~Array() {
    delete[] array;
}

template <class T>
void Array<T>::operator=(const T &temp) {
    for(int i = 0; i < length; i++) {
        array[i] = temp;
    }
}

template <class T>
void Array<T>::print() const {
    for(int i = 0; i < length; i++) {
        std::cout << array[i] << "\t";
    }
}

template <class T>
bool Array<T>::isEmpty() const {
    return (length == 0);
}

template <class T>
bool Array<T>::isFull() const {
    return (length == maxSize);
}

template <class T>
int Array<T>::maxListSize() const {
    return maxSize;
}

template <class T>
int Array<T>::listSize() const {
    return length;
}

template <class T>
T Array<T>::front() const {
    return array[0];
}

template <class T>
T Array<T>::back() const {
    return array[length-1];
}

template <class T>
void Array<T>::swap(T& x, T& y) {
    T temp = x;
    x = y;
    y = temp;
}

template <class T>
void Array<T>::sort() {
    for(int i = 0; i < length - 1; i++) {
        for(int j = 1; j < length - i - 1; j++) {
            if(array[j] > array[j-1]) {
                std::swap(array[j], array[j-1]);
            }
        }
    }
}

template <class T>
int Array<T>::search(const T& item) {
    for(int i = 0; i < length; i++) {
        if(array[i] == item){
            return i;
        }
        else {
            return 0;
        }
    }
}

template <class T>
void Array<T>::insert(int location, const T& item) {
    if(length != maxSize) {
        int i = length;

        for(;i > location; i--) {
            array[i] = array[i-1];
        }
        array[i] = item;
        length++;
    }

}

template <class T>
void Array<T>::insertAtTop(const T& item) {
    if(length != maxSize) {
        array[length] = item;
        length++;
    }
}

template <class T>
void Array<T>::deleteItem(const T& item) {
    bool flag = false;
    int location = 0;
    for(;location < length; location++) {
        if(array[location] == item) {
            flag = true;
            break;
        }
    }

    if(flag) {
        for(int i = location; i < length - 1; i++) {
            array[i] = array[i+1];
        }
        length--;
    }
    else {
        std::cout << "Element not found" << std::endl;
    }
}




#endif /* Array_h */

Here is the main file that tests some of the member functions (I have checked all and they work).

#include <iostream>
#include "Array.h"

int main(int argc, const char * argv[]) {

    Array<int> arr(5);
    arr.insertAtTop(2);
    arr.insertAtTop(4);
    arr.insertAtTop(6);
    arr.insertAtTop(8);
    std::cout<<"\n------------------------------------------------------------\n";
    std::cout<<"---------------Displaying All contents of array---------------";
    std::cout<<"\n------------------------------------------------------------\n";
    arr.print();

    std::cout<<"\n--------------------------------------------------\n";
    std::cout<<"-------------Inserting At Particular--------------";
    std::cout<<"\n--------------------------------------------------\n";
    arr.insert(1, 12);
    arr.print();

    std::cout<<"\n--------------------------------------------------\n";
    std::cout<<"----------------Deleting an Item-----------------";
    std::cout<<"\n--------------------------------------------------\n";
    arr.deleteItem(12);
    arr.print();

    return 0;
}

Answer 1

Implementing from scratch to learn about it is a worthy approach and I see that as the subject of presentations at the major conferences.

You should use the naming convention from the standard containers. You wrote a vector, not an array. You don’t insertAtTop, you push_back, etc.

---

T *array;

The style in C++ is to put the * or & with the type, not the identifier. This is called out specifically near the beginning of Stroustrup’s first book, and is an intentional difference from C style.

⧺C.149 — no naked new or delete.

You should probably make this a unique_ptr as a drop-in replacement without otherwise changing the architecture.

---

Array(int size = 100);

Make that explicit. Do you know why?

---

void print() const;

No, don’t make that a member function. Provide a general way for users to access the elements. Then a printing function can be written using the public API.

---

void swap(T& x, T& y);

Huh? That is a (non-static) member, so why is it taking two additional parameters?

Skipping down to look at the implementation: you wrote it as a non-member. So it should be a free (template) function swap.

As for how/whether it should be implemented, I’ll get back to that.

---

void sort();

No, just let the user do std::sort with your container or portions thereof.

int search(const T& item);

Again, can your implementation do better than a linear search? Just let the user use std::find and std::find_if on your collection!

---

void operator=(const T& temp);

It is not normal to have a void return here. You should return *this;.

Edit: OK, that’s not a copy assignment operator at all. void is fine. But the automatically-generated copy assignment code will not work right, as described next for the copy constructor.

---

Where is your copy constructor? Your class will seriously malfunction if you write something like:

Array<int> arr(5);
  ⋮
Array<int> a2 = arr;

See The rule of three/five/zero.

---

You should provide move semantics for efficient usage of the array object, and for faster insert/delete etc. when T is a complex type.

---

template <class T>
Array<T>::Array(int size) {
    if (size <= 0) {
        std::cout << "Size cannot be less than 0";

        maxSize = 100;
        array = new T[maxSize];
    }
    else {
        maxSize = size;
        length = 0;
        array = new T[maxSize];
    }

}

Don’t complain and use a different value. Treat it as an error.

if (size <= 0)  throw std::invalid_argument ("Array size negative");

You are actually constructing the full number of elements of type T. So your length is what? How many of them you care about? Normally, a collection will not construct elements that are not used.

---

template <class T>
Array<T>::~Array() {
    delete[] array;
}

If you used a unique_ptr like I suggested above, you would not need to write a destructor at all! (And, it would not automatically generate a bad copy constructor!)

---

template <class T>
void Array<T>::operator=(const T &temp) {
    for(int i = 0; i < length; i++) {
        array[i] = temp;
    }
}

Hmm, that’s not an assignment operator at all. It assigns a common value to all elements.

Just use std::fill_n.

---

The various trivial implementations should be in-line in the class definition.

---

template <class T>
void Array<T>::sort() {
    for(int i = 0; i < length - 1; i++) {
        for(int j = 1; j < length - i - 1; j++) {
            if(array[j] > array[j-1]) {
                std::swap(array[j], array[j-1]);
            }
        }
    }
}

A bubble sort? Are you kidding?!

---

Your front and back functions don’t check for zero length. You have a deleteItem function, so the user could delete all of them!

Speaking of deleteItem, you seem to duplicate the find code first. Just use find!

---

insert functions silently do nothing if the container is full. They should throw an exception.

Copying all the elements down one spot in a loop — can you find a std algorithm for that instead?