1

I am trying to create my own array within a class, with functions to insert into it, delete etc.. My array has capacity - max array's size, size - how many elements it holds, *data - a pointer to data. So when the user tries to insert an element and the array is full, the capacity would double in my resize() function, i would create a temporary array newData[capacity] copy everything there, then delete my original data to get rid of that memory and then assign newData to data. Now I don't know if that is a stupid solution, but it works the first time, but when I resize the second time I am getting strange numbers. For the tests I put the starting capacity to 2. This is myArray.cpp file:

#include <iostream>
#include "myArray.h"

using namespace std;

myArray::myArray()
{
    size = 0;
    capacity = 2;
    data = new int[capacity];
}

void myArray::setData(int n, int idx) {
    data[idx] = n;
}
int myArray::getData(int idx) {
    return data[idx];
}

void myArray::insert(int num) {
    size++;
    if(size > capacity) resize();
    setData(num, size - 1);
}
void myArray::insert(int num, int idx) {
    if(idx == size + 1) insert(num);
    else {
        size++;
        if(size > capacity) resize();
        for(int i = size; i > idx; i--) {
            data[i] = data[i - 1];
            if(i - 1 == idx) data[idx] = num;
        }
    }
}
void myArray::remove(int idx) {
    if(idx == size) {
        delete &data[size];
        size--;
    }
    else {
        for(int i = idx; i < size; i++) {
            data[i] = data[i+1];
        }
        size--;
    }
}
void myArray::resize() {
    cout << "Resizing" << endl;
    capacity *= 2;
    int *newData = new int[capacity];
    for(int i = 0; i < size; i++) {
        newData[i] = data[i];
    }
    delete[] data;
    data = newData;
    delete[] newData;
}

int& myArray::operator[](int idx) {
    return data[idx];
}

void myArray::print() {
    for(int i = 0; i < size; i++) {
        cout << data[i] << " ";
    }
    cout << endl;
}

myArray::~myArray()
{
    //dtor
}

Just ignore all of the functions I guess, all of the circus must happen in the resize() function. This is the header file

#ifndef MYARRAY_H
#define MYARRAY_H


class myArray
{
    public:
        myArray();
        virtual ~myArray();

        void print();

        void setData(int n, int idx);
        int getData(int idx);

        void insert(int num);
        void insert(int num, int idx);
        void remove(int idx);
        void resize();

        int &operator[](int);

    protected:

    private:
        int size;
        int capacity;
        int *data;
};

#endif // MYARRAY_H

And this are my tests in main()

#include <iostream>
#include "myArray.h"

using namespace std;

int main()
{
    myArray array;
    array.insert(1);
    array.print();
    array.insert(4);
    array.print();
    array.insert(3);
    array.print();
    array.insert(5, 3);
    array.print();
    array.remove(1);
    array.print();
    array.insert(6);
    array.print();
    array[2] = 2;
    array.print();
    array.insert(3, 0);
    array.print();
    return 0;
}

And this is what I see in the ouput:

1
1 4
Resizing (everything worked fine)
1 4 3
1 4 3 5
1 3 5
1 3 5 6
1 3 2 6
Resizing (everything is not fine)
3 18248184 18219200 2 6
Aldomond
  • 171
  • 6
  • [What is a debugger and how it can it help me diagnose problems?](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) – PaulMcKenzie Mar 24 '21 at 19:10
  • Also, if you are going to be doing manual memory allocations inside your class, You need to at least follow the rule of three: https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three – NathanOliver Mar 24 '21 at 19:11
  • `for(int i = 0; i < size - 1; i++) ` -- Let's say the `size` is 10. That loop only iterates 9 times. What happened to the last value? – PaulMcKenzie Mar 24 '21 at 19:13
  • 1
    Here's your program when executed after having been compiled with [`-fsanitize=address -lasan -lubsan`](https://gcc.godbolt.org/z/53hMj8Mqa) – Ted Lyngmo Mar 24 '21 at 19:13
  • @PaulMcKenzie Yes didn't notice that, thank you I fixed it to ```for(int i = 0; i < size; i++)``` – Aldomond Mar 24 '21 at 19:19
  • Have a read of this series on the common mistakes made when creating a vector class: https://lokiastari.com/series/ – Martin York Mar 24 '21 at 19:57
  • https://codereview.stackexchange.com is a good place to get your code reviewed now that it is working. – Martin York Mar 24 '21 at 19:57

1 Answers1

1

In resize, the delete[] newData; statement deletes the memory you just allocated, leaving data as a dangling pointer because it now points at memory that has been deallocated.

The solution is to remove the delete[] newData; statement from resize.

You should also add code to the destructor to free up the memory you've allocated.

1201ProgramAlarm
  • 32,384
  • 7
  • 42
  • 56
  • Oh, this actually did solve the problem, but it works for me one time and doesn't the another time. I deleted the delete[] newData, and I also added data = new int[capacity]; after the delete[] data; So now one time I compile it outputs 3 1 3 2 6 like it should, but the other time it is stuck on the word Resizing in the ouput and nothing happens. What is this magic, how can it work one time and it won't the another time? – Aldomond Mar 24 '21 at 19:53
  • @Norbertas Why are you assigning again to `data`? You've already did that with `data = newData`, so that the newly allocated memory will be retained and used. – 1201ProgramAlarm Mar 24 '21 at 19:55
  • Sorry my bad on that one. But still, if I leave only the delete[] data; and then data = newData; most of the time it prints the normal output, but one in 6 tries It says Resizing and doesn't print the array. – Aldomond Mar 24 '21 at 19:57
  • @Norbertas There are at least 2 issues in `remove` that might contribute to that. – 1201ProgramAlarm Mar 24 '21 at 20:00
  • I figured in my loop (int the ```remove``` function), when I am removing elements and pushing all of them to the left side, when I get to the last element I am trying to assign the last element's value to the value of the element that is on it's right, but there is nothing there so it gets assigned to some trash. But even if I fix that it still won't always print the output.. I guess I am just too confused – Aldomond Mar 24 '21 at 20:16