0

I was making a std::vector like clone for a fun little project in C++, and I've come across the following problem: I'm trying to implement the method pop_back but for some reason, I can't get it to work. The class I'm using is called Array and is defined in ../d_arrays/d_array.h and ../d_arrays/d_array.cpp I have a main.cpp file in the root folder. Here is what I have so far:

RootDir/d_arrays/d_array.h

#include <iostream>

using namespace std;

class Array {

    public:

        Array(int len);

        void pop_back();
        void print();

    private:
        
        int* _array = NULL;
        int _len = 0;

};

RootDir/d_arrays/d_array.cpp

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

using namespace std;

Array::Array(int len) {
    _array = new int[len];
    _len = len;
}

void Array::pop_back() {
    _len--; // reduce length by 1

    int* tmp = new int[_len]; // create temp copy

    for (int i = 0; i < _len; i++) {
        tmp[i] = _array[i];
    }

    _array = tmp;
    delete[] tmp;
}

void Array::print() {
    for (int i = 0; i < _len; i++) {
        cout << _array[i] << " ";
    }
    cout << endl;
}

RootDir/main.cpp

#include <iostream>

#include "d_arrays/d_array.h"

using namespace std;

int main() {

    Array a = Array(10);

    cout << "Before: ";
    a.print();
    a.pop_back();
    cout << "After: ";
    a.print();

    return 0;

}

Im using g++ -o main main.cpp d_arrays/d_array.cpp to compile my code and ./main to run it.

whenever I run ./main I get the following output: Before: 0 0 0 0 0 0 0 0 0 0 After: 0 0 118968336 21854 0 0 0 0 0 What is happening here?

7
  • 3
    1. I would seriously suggest not resizing arrays for every pop_back. 2. The array's contents are never initialized in the initial array, thus, using any of those values is undefined. 3. You also delete[] tmp which is the new array you just allocated. Commented Feb 17, 2022 at 15:16
  • 3
    try std::swap(_array, tmp); instead of _array = tmp; Commented Feb 17, 2022 at 15:17
  • 2
    Your problem is here: _array = tmp; delete[] tmp; You deleted the new array since both tmp and _array point to the same memory. Commented Feb 17, 2022 at 15:17
  • 1
    len--; // reduce length by 1 -- And that is all you really need to do. All of that other code that attempts to resize the memory allocated is not necessary. Commented Feb 17, 2022 at 15:21
  • 3
    @Verve -- If your goal is to make a vector-like clone, pop_back() is supposed to be a constant-time operation. Your pop_back() has linear time complexity, all due to the creation of memory and recopying. Imagine if the vector had a million elements -- pop_back() would go through the gauntlet of decreasing the length (ok), but then reallocate 999,999 elements again and initialize them with data that you just deallocated. That does not make a lot of sense, especially since _len is what controls the number of entries. Commented Feb 17, 2022 at 15:29

1 Answer 1

2

Your problem is here:

delete[] tmp;

Now:

_array = tmp

means:

Point _array to the memory address that tmp is pointing to.

Now, _array is pointing to &tmp (& means 'address of'), and then you delete tmp, which causes issues.

Also resizing the entire array would be ok if the array is small. But it would not be ok if the array is big. For example, if the array contains millions of elements, resizing the array is going to take a long time. So you can just not resize the array, as _len is what handles the number of elements.

So the new pop_back function can be as follows:

void Array::pop_back() 
{
    _len--; // reduce length by 1

    /* int* tmp = new int[_len]; // create temp copy

    for (int i = 0; i < _len; i++) 
    {
        tmp[i] = _array[i];
    }

    _array = tmp; 
    delete[] tmp; */
}

As you can see I've commented out all code except _len--;

But if you really want to resize the array at some point, you can declare a separate function for that as such:

void Array::shrink_to_fit()
{
    int* tmp = new int[_len]; // create temp copy

    for (int i = 0; i < _len; i++)
    {
        tmp[i] = _array[i];
    }

    delete[] _array; // Delete what _array is pointing to
    _array = tmp; // Point _array to the memory address that tmp is pointing to
}

But with this new function, don't forget to define shrink_to_fit() in your class definition. So the new class definition:

class Array 
{
public:

    Array(int len);

    void pop_back();
    void print();
    void shrink_to_fit();

private:

    int* _array = NULL;
    int _len = 0;
};
Sign up to request clarification or add additional context in comments.

1 Comment

I really appreciate the answer and explanation!

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.