-1

I'm trying to implement a function that returns an array, I came to this solution, but I don't know if it is a good practice, that's how I did it:

#include <iostream>
using namespace std;

int* returnNewArray(int n) {
    int* arr = new int[n];

    for (int i=0;i<n;i++)
        arr[i] = i;
    
    return arr;
}

int main() {
    int n = 5;
    int* arr = returnNewArray(n);

    for (int i=0;i<n;i++)
        cout << arr[i] << "\t";
    
    delete[] arr;
    arr = NULL;

    cout << endl;

}

I wonder if it is necessary to deallocate the memory that I allocated in the function to create the dynamic array (arr).

  • If you call new[] somewhere you should call delete[] on the same pointer when you are done with the memory. With that said when your program ends your OS will release all memory allocated. – drescherjm Sep 07 '22 at 18:44
  • 9
    No, it's not good practice. Return a `std::vector`. [Example](https://godbolt.org/z/n5jErdfWd) – Ted Lyngmo Sep 07 '22 at 18:44
  • Yes that's exactly what you're supposed to do. You can also try smart pointers which will do this automatically for you – smac89 Sep 07 '22 at 18:44
  • To see your options, I suggest you read this: https://stackoverflow.com/questions/16711697/is-there-any-use-for-unique-ptr-with-array – BitTickler Sep 07 '22 at 18:48
  • @BitTickler I updated [this answer](https://stackoverflow.com/a/24852984/7582247) with the C++20 option I used in my answer. – Ted Lyngmo Sep 07 '22 at 19:59
  • 2
    This might seem pedantic, but actually it important to properly understand the difference, You are not returning an array (that's actually impossible in C++) you are returning a pointer (which in this case happens to point to a dynamically allocated array). – john Sep 07 '22 at 20:20

1 Answers1

3

I don't know if it is a good practice

It's not. Nowadays, cases where using new/new[] and delete/delete[] are necessary are very few.

I wonder if it is necessary to deallocate the memory that I allocated in the function

It is necessary if you want to avoid memory leaks and since you used a raw owning pointer, you need to do it manually, just like you do in your code. Your code is cleaning up correctly.

Good practice would however be to use a std::vector<int> or at least use a smart pointer like std::unique_ptr<int[]> instead since these will clean up memory automatically when they go out of scope.

vector<int> version:

#include <numeric> // std::iota
#include <vector>  // std::vector

std::vector<int> returnNewArray(size_t n) {
    std::vector<int> arr(n);
    std::iota(arr.begin(), arr.end(), 0);     // [0, n)
    return arr;
}

unique_ptr<int[]> version:

#include <memory>  // std::unique_ptr / std::make_unique_for_overwrite
#include <numeric> // std::iota

std::unique_ptr<int[]> returnNewArray(size_t n) {
    auto arr = std::make_unique_for_overwrite<int[]>(n);
    std::iota(arr.get(), arr.get() + n, 0);   // [0, n)
    return arr;
}

Both versions will let you iterate over the result just like you do in your code - and you don't have to delete[] anything when you're done:

auto arr = returnNewArray(n);
    
for(int i = 0; i < n; ++i)
    std::cout << arr[i] << '\t';  // 0  1  2  3  4

But the std::vector<int> has the benefit of knowing its own size and can be used in range based for-loops, which also helps to not accidentally access the array out-of-bounds:

for (int value : arr)             // range based for-loop
    std::cout << value << '\t';   // 0  1  2  3  4
Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108