Home > Software design >  Dynamic Array - Problem with memory management
Dynamic Array - Problem with memory management

Time:10-19

I'm working on the dynamic array. Related part of code of the array class:

#pragma once
#include <iostream>

template <class T>
class Darray
{

private:
    T* dataArray;
    int a_size = 0;
    int a_capacity = 0;
    double expandFactor = 1.5;

private:
    void memLoc(int n_capacity)
    {
        T* newArray = new T[n_capacity];

        for (int i = 0; i < a_size; i  )
        {
            newArray[i] = dataArray[i];
        }

        
        dataArray = newArray; 
        delete[] newArray; //<-- **************problem occurs here**************
        a_capacity = n_capacity;
        
        
    }

public:
    Darray()
    {
        T* dataArray = new T[a_size];
        memLoc(2);
    }

    void addLast(T data) 
    {
        if (a_size < a_capacity)
        {
            dataArray[a_size] = data;
            a_size  ;
        }
        else
        {
            memLoc(a_capacity * expandFactor);
            dataArray[a_size] = data;
            a_size  ;
        }
    }

When I run the code without deleting the newArray I get expected result:

Values
Index 0: 10
Index 1: 9
Index 2: 8
Index 3: 7
Index 4: 6
Index 5: 5
Index 6: 4
Index 7: 3
Index 8: 2
Index 9: 1

Here is my problem! When I delete newArray (marked in the code) my results are far from accurate:

Values
Index 0: -572662307
Index 1: -572662307
Index 2: 100663302
Index 3: 31812
Index 4: 17648624
Index 5: 17649832
Index 6: -572662307
Index 7: -572662307
Index 8: -572662307
Index 9: -572662307

I have no idea why this is happening because at first glance everything seems to be correct. Any suggestion or hint would be appreciated. I hope somebody will be able to help ;)

CodePudding user response:

There are a couple of mistakes in your code.

  • In memLoc(), you are destroying the new array you just created. You need to instead destroy the old array that is being replaced. The statement dataArray = newArray; is just assigning a pointer to another pointer. dataArray is pointing at the previous array, and newArray points at the new array. When you perform dataArray = newArray;, you leak the previous array, and now both dataArray and newArray are pointing at the same array, and then delete[] newArray; destroys that array, leaving dataArray as a dangling pointer to invalid memory.

  • In your default constructor, you are creating a new array and assigning its pointer to a local variable named dataArray, which is hiding the class data member also named dataArray. You are then calling memLoc() to create another array for the class data member dataArray to point at. The 1st array is useless and gets leaked. You should instead initialize the class data member to nullptr, and just call memLoc() to create the only array.

Also, while these are not mistakes per-say, they are noteworthy:

  • I'm assuming the rest of the code you have not shown is compliant with the Rule of 3/5/0, ie you have a destructor, copy/move constructors, and copy/move assignment operators. If not, make sure you address that, otherwise you will run into other problems later.

  • the code in addLast() is repetitive and can be simplified.

With that said, try this:

#pragma once
#include <iostream>
#include <utility>

template <class T>
class Darray
{
private:
    T* dataArray = nullptr;
    int a_size = 0;
    int a_capacity = 0;
    static const double expandFactor = 1.5;

private:
    void memLoc(int n_capacity)
    {
        T* newArray = new T[n_capacity];

        for (int i = 0; i < a_size;   i)
        {
            newArray[i] = dataArray[i];
        }

        delete[] dataArray;
        dataArray = newArray; 
        a_capacity = n_capacity;
    }

public:
    Darray()
    {
        memLoc(2);
    }

    Darray(const Darray &src)
    {
        memLoc(src.a_capacity);
        for (int i = 0; i < src.a_size;   i)
        {
            dataArray[i] = src.dataArray[i];
              a_size;
        }
    }

    Darray(Darray &&src)
    {
        std::swap(dataArray, src.dataArray);
        std::swap(a_capacity, src.a_capacity);
        std::swap(a_size, src.a_size);
    }

    ~Darray()
    {
        delete[] dataArray;
    }

    Darray& operator=(Darray rhs)
    {
        std::swap(dataArray, rhs.dataArray);
        std::swap(a_capacity, rhs.a_capacity);
        std::swap(a_size, rhs.a_size);
        return *this;
    }

    void addLast(const T &data) 
    {
        if (a_size >= a_capacity)
            memLoc(a_capacity * expandFactor);
        dataArray[a_size] = data;
          a_size;
    }

    ...
};
  • Related