Hi and welcome to the site. Your code already looks quite good. However, there are still some issues beyond what @Björn Linqvist wrote.

1. const correctness

   This means that all variables as well as function arguments that are not mutated should be declaredconst. In addition methods that do not change the state of the object, aka pure observers should also be markedconst. This greatly improves the ability to reason about code and helps the compiler help you. As an example

   int size() const {    return _size;}T get(const int pos) const {    return array[pos];}

2. The latter function is an even better example as it shows that something is missing. Often insideconst code pathes you need to access elements of the array. So you need both aconst and a non-const accessor. Also you are returning acopy of the object. Generally, a reference is returned.

   T& get(const int pos) {    return array[pos];}const T& get(const int pos) const {    return array[pos];}

3. YourdeleteAt function is subtly incorrect.

   void deleteAt(int pos) {    assert(0 <= pos && pos < _size);    _size--;    for (int i = pos; i < _size; i++) {        array[i] = array[i + 1];    }}

   Here you are shifting the elements from[pos, size_ - 1] to the left. However, what happens whenT is a nontrivial type that has a destructor? The element at positionsize_ is copied to positionsize_ - 1 but is stillalive. You need to explicitely callstd::destroy here or you will get a lot of incredibly hard to debug bugs.

4. Honor conventional naming.

   C++ has a rich ecosystem of libraries and the STL. Most of these use consistent naming conventions, e.g.insert rather thaninsertAt. This might not seem a big deal but other programmers will have a hard time using your code. Even worse you will have a hard time using other code as you will mix those names.

   This is especially bad when naming contradicts expected behavior.resize conventionally takes a input argument that represents the size the container should have. Yourresize method does something different so it will be highly confusing.

5. Please do not useusing namespace std; This will at some point get you in all sorts of trouble. Maybe not with the STL but definitely when you use it with other namespaces as the actual functionality ofusing namespace foo is highly surprising. There is a reason namespaces are generally short and typingstd:: is quite easy.

6. Iterators.

   C++ algorithm use iterators as an incredibly powerfull abstraction. You should providebegin() andend() methods. Also you should understand why you would wantbegin() andbegin() const andcbegin().

7. Algorithms.

   C++ algorithms are incredibly powerfull and the STL is full of them. You should have a look at the algorithms and try to use them in you code (insert and erase are good candidates). I can highly recommend Connor Hoekstra and hisAlgorithm Intuition talk.

The code is pretty good as it is. Here is how I would improve it:

Constant handling

In C++, you should preferconst over#define. So

#define INITIAL_CAPACITY 5

becomes

const int INITIAL_CAPACITY = 5;

Also don't use all uppercase names for non constant variables. It isconfusing and breaks the 50 year old tradition.

Clearer names

I renamed a few of your variables:

• MIN_CAPACITY => capacity Because the instance variable holds thecurrent capacity of the dynamic array, not theminimum capacity.

• length() => size The wordslength andsize are synonymous butusing them both can cause confusion. A reader might think is thelength the number of elements in the array and size the allocactedsize, or vice versa?

Offby one errors

You have an offby one error indeleteAt. If you have a dynamic arraywith 5 elements, then their positions are 0 to 4 sodeleteAt(5)shouldn't work, but it does. I've fixed that for you.

Pretty printing

Adding pretty printing functions is almost always a good idea becausethey make debugging much easier. I've added one for you.

Error handling

Your error handling consists of printing to stdout and letting theprogram continue to run. That is incorrect because errors might not bediscovered if stdout is redirected or the user is not paying attentionto what is printed.

There are many ways to handle errors. I've implemented basic assertbased error handling for you. But you can certainly be fancier and useexceptions or status codes.

Functions implemented in terms of each other

For all dynamic arraysdynarr.append(x) is equivalent todynarr.insertAt(x, dynarr.size()). Soappend can just callinsertAt.

Destructor

Your dynamic array allocates memory in the constructor, but there isno corresponding destructor that frees the memory. I've added onelooking like this:

~dynamic_array() {    delete[] array;}

Copying memory

There's a function calledstd::copy which you can use for copyingmemory. That way, you don't have to write your own copy function.

Pointless comments

Writing good comments is hard. As a reviewer I much prefer no commentsover pointless comments. An example of a pointless comment is//constructor init. I can see that the lines below is the constructorso the comment doesn't tell me anything I didn't already know. Samefor the comment// returns size of array.

Source code

Here is the improved version of the dynamic array:

#include <assert.h>#include <cstring>#include <iostream>using namespace std;const int INITIAL_CAPACITY = 2;const int GROWTH_FACTOR = 2;template <class T>class dynamic_array {    T *array;    int capacity = INITIAL_CAPACITY;    int _size;public:    dynamic_array() {        array = new T[capacity];        _size = 0;    }    ~dynamic_array() {        delete[] array;    }    void deleteAt(int pos) {        assert(0 <= pos && pos < _size);        _size--;        for (int i = pos; i < _size; i++) {            array[i] = array[i + 1];        }    }    void insertAt(int element, int pos) {        assert(0 <= pos && pos <= _size);        if(_size == capacity) {            resize();        }        for(int i = _size; i > pos; i--) {            array[i] = array[i-1];        }        _size++;        array[pos] = element;    }    void append(T element) {        insertAt(element, _size);    }    int size() {        return _size;    }    // doubles capacity if it has to and deletes reference to current array.    void resize() {        capacity *= GROWTH_FACTOR;        T *temp = new T[capacity];        copy(array, array + _size, temp);        delete [] array;        array = temp;    }    T get(int pos) {        return array[pos];    }    void pretty_print() {        cout << "[";        for (int i = 0; i < _size - 1; i++) {            cout << array[i] << " ";        }        if (_size) {            cout << array[_size - 1];        }        cout << "]\n";    }};

• \$\begingroup\$Usingmemcpy here is not a good idea, and will behave badly if T isn't trivially copyable.\$\endgroup\$

  interjay

  – interjay

  2019-12-24 17:24:32 +00:00

  CommentedDec 24, 2019 at 17:24
• \$\begingroup\$True, it should bestd::copy\$\endgroup\$

  Gaslight Deceive Subvert

  – Gaslight Deceive Subvert

  2019-12-24 18:46:04 +00:00

  CommentedDec 24, 2019 at 18:46

• c++
• beginner
• algorithm
• array