trjhtr

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;

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?