C++ logical error with using dynamic arrays

Question

Inside the insert function, I use dynamic memory allocation with the arrays ditems and tempItems.

Where I am using tempItems to be an array twice as big as ditems and also store all the items from ditems temporary; and then deleting and assigning ditems equaling tempItems

The code complies fine, but when testing it with say enough data requiring ditems to store 2000 elements, it appears that the ditems array is not getting any bigger.However if I was to set (arrayCap=2001;) in the constructor, then there is no problem.

I am new to using dynamic arrays and looking at other code using dynamic arrays, it doesn't look like I had made any mistakes. I can't use vectors for this task, so I am stuck with dynamic arrays, but I am not sure what is wrong here.

template <class T>
class Vector {
    public:
        typedef T* iterator;
        Vector () { 
            arrayCap=1000;
            ditems = new T[arrayCap];
        }

        T& operator[](unsigned int i) {
           return ditems[i];
        }

        iterator begin () {
            used=0;
            return &ditems[used];
        }
        iterator end () { 
            return &ditems[used];
        }
        int size () { return used; }
        void deletes(){

            used--;

        }

        iterator insert (iterator position, const T& item) { 

            if(arrayCap-used<100)
            {
                temp=arrayCap;
                arrayCap=2*arrayCap;

                tempItems=new T[arrayCap];
                for(int i=0; i<temp;i++)
                {
                    tempItems[i]= ditems[i];
                }
                delete [] ditems;
               ditems=tempItems;  
            }

            for(Vector<T>::iterator i=&ditems[arrayCap-1]; i>position; i--)
            {
               *i=*(i-1);
            }
            used++;
            *position= item;
            return position;
        }
    private:
        int arrayCap,temp;
        T *ditems;
        T *tempItems;
        int used;

};

Your begin() and end() return the same thing. Your range will always appear empty. — François Andrieux
– François Andrieux, Commented Sep 14, 2018 at 13:29
tempItems should be a local variable in the insert method. There's no reason to make it a class variable. Same is true of the temp variable as well. — john
– john, Commented Sep 14, 2018 at 13:30
Use of the used variable is incorrect, it should be set in the constructor not in the begin method. The begin method should be return &ditems[0]; — john
– john, Commented Sep 14, 2018 at 13:33
@FrançoisAndrieux not in this case, within other sections of my code not included here, I only call begin() once per creation of an object, so used will increase when I insert more elements into ditems. — user9111757
– user9111757, Commented Sep 14, 2018 at 13:35
@StackUser when you call your begin method you set used to zero. In other words calling begin sets the size of your vector to zero. Obviously that's a bug. — john
– john, Commented Sep 14, 2018 at 13:37

mch · Accepted Answer · 2018-09-14 14:18:55Z

2

Moving the array to a new position invalidates the iterator position, it points to the old array. So i>position is undefined behavior.

You should calculate the index before moving the array and set position at the index in the new array.

template <class T>
class Vector {
    public:
        typedef T* iterator;
        Vector () { 
            arrayCap=1000;
            ditems = new T[arrayCap];
            used = 0;
        }

        T& operator[](unsigned int i) {
           return ditems[i];
        }

        iterator begin () {
            return ditems;
        }
        iterator end () { 
            return &ditems[used];
        }
        int size () { return used; }
        void deletes(){
            used--;
        }

        iterator insert (iterator position, const T& item) { 

            if(arrayCap-used<100)
            {
                auto temp=arrayCap;
                arrayCap*=2;
                auto index = position - ditems;

                auto tempItems=new T[arrayCap];
                for(int i=0; i<temp;i++)
                {
                    tempItems[i]= ditems[i];
                }
                delete [] ditems;
                ditems = tempItems;
                position = ditems + index;
            }

            for(Vector<T>::iterator i=&ditems[arrayCap-1]; i>position; i--)
            {
               *i=*(i-1);
            }
            used++;
            *position = item;
            return position;
        }
    private:
        int arrayCap;
        T *ditems;
        int used;
};

edited Sep 14, 2018 at 14:18

answered Sep 14, 2018 at 13:29

mch

9,8243 gold badges33 silver badges46 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

user9111757 Over a year ago

I changed the array before it goes through that loop so shouldn't it point to the new ditems array

mch Over a year ago

ditems will point to the new array, but position (the function parameter) does not.

user9111757 Over a year ago

I think I kind of understand, but then how do I get position to point to the new array ditems

mch Over a year ago

int index = position - ditems; before moving the array and position = ditems + index; afterwards. I added the edited code snippet in the answer.

Collectives™ on Stack Overflow

C++ logical error with using dynamic arrays

1 Answer 1

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related