1

I want to create a vector of data, but I want to both set it's size and fill its elements in a sub-function. Is this an appropriate time to use the new operator? Is there a better way to do it? It seems like an appropriate time, but I'm hesitant because of Why should C++ programmers minimize use of 'new'?

int main()
{
    vector<double> *array1;
    vector<double> *array2;
    OtherArgs otherArgs;
    FillArrays(array1,array2,otherArgs);
    //Do other stuff 
    delete array1;
    delete array2;
}

void FillArrays(vector<double> *&array1, vector<double> *&array2, OtherArgs &otherArgs)
{
    int size=GetSize(otherArgs);
    array1 = new vector<double>(size);
    array2 = new vector<double>(size);
    //Other code to fill the arrays
}

Thank you

Community
  • 1
  • 1
user1415670
  • 91
  • 1
  • 1
  • 3
  • 2
    Why don't you just return a `tuple`/`pair` of `vector`s? – K-ballo May 30 '12 at 18:54
  • There's nothing wrong with using new but it doesn't seem like you need to. – evanmcdonnal May 30 '12 at 18:54
  • @K-ballo: You should write that up as an answer. The pair solution is easier to use ("fluent"), and easier to understand, and if you're using C++11, or a C++98 compiler with a good optimizer, it will produce code with the same performance as JaredPar's answer. Of course if any of the compilers you need to support end up copying the contents of the vectors (and the time taken to do that makes a difference), then you can fall back to JaredPar's answer as an optimization. – abarnert May 30 '12 at 19:19

2 Answers2

7

Here are a couple of reasons why the original sample is troublesome

  • The code leaks memory in the face of an exception because the delete calls aren't protected
  • If FillArrays is passed a non-NULL vectory<double> value it will leak memory because it didn't delete the previous value. It couldn't reliably call delete even if it wanted to because the value may have been stack allocated.

The easiest way to do this is to just declare the values on the stack and pass them by reference that way.

int main()
{
    vector<double> array1;
    vector<double> array2;
    OtherArgs otherArgs;
    FillArrays(array1,array2,otherArgs);
    //Do other stuff 
}

void FillArrays(vector<double> &array1, vector<double> &array2, OtherArgs &otherArgs)
{
    int size=GetSize(otherArgs);
    //Other code to fill the arrays
}

The vector<T> will initialize themselves to an empty list when declared in this manner. The FillArrays method can then populate them as necessary.

JaredPar
  • 673,544
  • 139
  • 1,186
  • 1,421
3

No, this is a specifically bad use of new.

If you must use new and delete1, use them as bookends. The delete expression should be in a logically consistent context with the new expression.

So, if you new in a c-tor, you should delete in the d-tor. If you new in an allocation function, you should delete in a deallocation function. The two calls should be arranged in an way that makes it obvious that one starts an operation and the other finishes it.

This implies that the call to new and delete should exist in the same layer in a functional or object hierarchy. Specifically, new and delete should be seen as implementation details, and not as part of the API contract.2

In your case, the new and delete are in entirely distinct contexts. new is inside the implementation, while delete is called for in the client. This will lead to programming errors.

Beyond that, the non-new version of your code (creating empty vectors in main, and passing them by reference) is simpler, easier to read, and more exception-friendly than the new version.

No, this is not a good example of when to use new. It is, however, an excellent example of when not to.


1 You hardly ever need to use new and delete. If you need dynamic allocation, just let containers keep copies of your objects.

2 std::shared_ptr, et al, violate this rule -- the new and delete are specifically part of the contract. This is probably OK, since pointer manipulation is their reason for existence. But SO has seen its share of bugs about storing a non-new pointer in shared_ptr.

Robᵩ
  • 143,876
  • 16
  • 205
  • 276