push_back and insert do not work properly in my own vector class

Question

So, here is the header file where I implemented my own vector class in a simple way. The problem is that even though the code compiles, the member functions pushBack and insert both do not work properly.

I'd be really glad if anyone can figure out the problem in my code or find some ways how I can get this problem resolved.

#include <iostream>
#include <new>
using namespace std;

template <typename T>
class DiyVector
{
  public:
    DiyVector()
    {
        arr = new T[1];
        arrSize = 0;
        index = 0;
        capacity = 1;
    }
    ~DiyVector()
    {
        delete[] arr;
    }

    T& at(unsigned int index) const
    {
        if (index >= arrSize || index < 0)
        {
            throw OutOfRange{};
        }
        else
        {
            return arr[index];
        }
    }

    unsigned int size() const
    {
        return arrSize;
    }

    void pushBack(const T& item)
    {
         if (arrSize == capacity)
         {
            arrTemp = new T[capacity += 1];

            for (unsigned int i = 0; i < capacity; ++i)
            {
                arrTemp[i] = arr[i];
            }

            delete[] arr;
            capacity++;
            arr = arrTemp;
         }

         arr[arrSize] = item;
         arrSize++;

    }

    void popBack()
    {
        if (arrSize == 0)
        {
            throw OutOfRange{};
        }
        else
        {
            arrSize--;
        }
    }

    void erase(unsigned int index)
    {
        if (index >= arrSize || index < 0)
        {
            throw OutOfRange{};
        }

        else
        {
            arrTemp = new T[capacity -= 1];

            for (unsigned int i = 0; i < capacity; ++i)
            {
                arrTemp[i] = arr[i];
            }

            delete[] arr;
            capacity--;
            arr = arrTemp;
        }
    }

    void insert(unsigned int index, const T& item)
    {
        if (index >= arrSize || index < 0)
        {
            throw OutOfRange{};
        }

        else if (index == capacity)
        {
            pushBack(item);
        }
        else if (0 <= index && index <= size())
        {
            arr[index] = item;
        }
    }

    class OutOfRange{};

private:
    unsigned int arrSize;
    unsigned int capacity;
    unsigned int index;
    T* arr;
    T* arrTemp;
  };

did you mean `capacity += 1;` instead of `capacity + 1;` in `push_back`? — fas, Oct 23 '19 at 11:06
I'm creating a new array that can exactly hold one more element. That's why I used capacity + 1 — izolveidon, Oct 23 '19 at 11:07
@debrisoroids `capacity + 1` does nothing, because you don't assign the value to anything. So you have to save `capacity = capacity + 1` or in short `capacity += 1` — RoQuOTriX, Oct 23 '19 at 11:09
Okay I changed it now but the problem still lies the same :( — izolveidon, Oct 23 '19 at 11:11
`index` is never initialized in the constructor, so using it becomes undefined behavior. There are multiple fundamental bugs in the shown code; and likely other bugs too, that a complete analysis will uncover. You need to learn how to use a debugger, so you can debug your own code and see everything that's wrong with it. stackoverflow.com isn't really a substitute for your own debugger. This is what a debugger is for. Knowing how to use a debugger is a required skill for every C++ developer. Have you yet used a debugger with this code, and if not, why not? — Sam Varshavchik, Oct 23 '19 at 11:11
Thank you for the suggestion. I'm really new to coding. I'm going to take your advice and improve my code. — izolveidon, Oct 23 '19 at 11:15
And why exactly "`if (index >= arrSize || index < 0)`" appears in nearly every method, even this one, for which it makes absolutely no sense? This looks like something that was robo-copied into every method, without fully understanding the need for it; then, to make the compilation error go away `index` gets added as a class member; but without fully understanding the reason for the compilation error. When you're looking at a compilation error you don't understand, the right thing to do is to figure it out, not search for it on the Internet, and assume that the first explanation, applies here. — Sam Varshavchik, Oct 23 '19 at 11:19
@debrisoroids If you are new to coding, I wouldn't recommend to start with an implementation of vector. Real vectors are quite complicated and involve advanced beasts such as _placement new_, `std::move_if_noexcept`, etc. There are even discussions about whether vectors are implementable according to C++ rules (e.g., treating objects created in an uninitialized memory as arrays). Even your implementation will have big problems with _exceptions_, which you do not seem to care about. — Daniel Langr, Oct 23 '19 at 11:19
@DanielLangr I'm taking a programming course on c++ and I was given this task to complete. I know my code is still not neat but I'm learning still. — izolveidon, Oct 23 '19 at 11:26
@debrisoroids All right, this make sense (though I don't like such assignments that basically force you to implement a very bad version of vector). Anyway, note that `index` cannot be `<0` when its type is `unsigned int`. — Daniel Langr, Oct 23 '19 at 11:31
A "programming course on C++" is unlikely to be starting with templates, and other advanced topics, right from the beginning. By the time I started learning templates, I no longer described myself as "new to coding". — Sam Varshavchik, Oct 23 '19 at 12:30
@SamVarshavchik Then that could possibly mean I'm weak at coding, which I don't deny as well but hey, it's not like I'm not trying to overcome my weak sides of programming. — izolveidon, Oct 23 '19 at 12:36
Possible duplicate of [How do I use arrays in C++?](https://stackoverflow.com/questions/4810664/how-do-i-use-arrays-in-c) — izolveidon, Oct 23 '19 at 18:47

Vlad from Moscow · Accepted Answer · 2019-10-23T12:18:13.577

2

For starters this check in some member functions (for example including insert)

if (index >= arrSize || index < 0)
                        ^^^^^^^^^^

does not make sense because the variable index declared as having the type unsigned int never can be negative. So you may exclude the second sub-condition of the if statement.

In the function pushBack in this loop

        for (unsigned int i = 0; i < capacity; ++i)
        {
            arrTemp[i] = arr[i];
        }

subsritute capacity for arrSize.

Also remove the statement

capacity++;

because capacity was already incremented in this statement

arrTemp = new T[capacity += 1];

Within the function insert change this code snippet

    else if (index == capacity)
    {
        pushBack(item);
    }
    else if (0 <= index && index <= size())
    {
        arr[index] = item;
    }

to

    if ( arrSize <= index )
    {
        pushBack(item);
    }
    else
    {
        arr[index] = item;
    }

And you may remove the data member

unsigned int index;

because it is not used.

Pay attention to that the member function erase is also incorrect. At least there is no need to reallocate the array and change its capacity. And again there are twice capacity is decremented.

     arrTemp = new T[capacity -= 1];
     //...
     capacity--;

that is the function has the same defects as the function pushBack. And in the loop you are coping the erased element.

edited Oct 23 '19 at 12:18

answered Oct 23 '19 at 11:47

Vlad from Moscow

301,070
26
186
335

I would add a note that this solution based on OP's code may increase capacity with each `pushBack`, which would be very inefficient. It would be better to, e.g., double the capacity instead. – Daniel Langr Oct 23 '19 at 11:56
1

@DanielLangr I updated the post relative to the redundant data member index. – Vlad from Moscow Oct 23 '19 at 11:59
but for the insert member function, I'm throwing OutOfRange error if (index >= arrSize) – izolveidon Oct 23 '19 at 12:01
1

@debrisoroids But that `index` refers to the parameter of `insert`, not to `index` member variable. – Daniel Langr Oct 23 '19 at 12:02
@DanielLangr I know about doubling the capacity but my task has this limitation where my task mentions that we have to increase the capacity exactly by one just to store one more element. – izolveidon Oct 23 '19 at 12:03
1

@debrisoroids But in any case the capacity shall not be decreased two times in the function erase. – Vlad from Moscow Oct 23 '19 at 12:04
@debrisoroids Ok, another strange requirement ;-) – Daniel Langr Oct 23 '19 at 12:05

push_back and insert do not work properly in my own vector class

1 Answers1