0

For a program of mine I made a small function to clear the various std::vectors of pointers that I have.

template <class S>
void clearPtrVector(std::vector<S*> &a,int size)
{
    for(size_t i = 0; i < size; i++)
         delete a[i];

    a.clear();
}

I must have done something wrong here though since when calling this function in a destructor like so :

clearPtrVector(neurons,neurons.size());

I get the following undefined reference two times:

undefined reference to `void clearPtrVector<Neuron>(std::vector<Neuron*,std::allocator<Neuron*> >&, int)'

I have to admit I am not familiar with what the std::allocator is, so I can not guess what the problem might be here. Any help is really appreciated. Thanks in advance!

-Lefteris

sharptooth
  • 167,383
  • 100
  • 513
  • 979
Lefteris
  • 3,196
  • 5
  • 31
  • 52
  • 3
    As a quick comment, you have a potential leak in your code. Why bother having a developer pass in the size? Instead just get it from the vector in the function. Otherwise you could have someone try to use this to clear just half the vector, and then the 'clear' call at the bottom will orphan memory. – workmad3 May 27 '09 at 07:59
  • Yeah you are right about that, I was about to do that just now, as I was looking at my code it just did not make sense :) – Lefteris May 27 '09 at 08:03

6 Answers6

8

Hot Fix

Write following instead:

  template <class Vector>
  void clearPtrVector(Vector &a)
  {
    for(size_t i = 0; i < a.size(); i++)
         delete a[i];

    a.clear();
  }

Make sure you define the template somewhere where compiler can see it before each use of the template. If you do not create a declaration, you should be safe, as otherwise you should get a compile error. If you do create a declaration for any reason, be double carefull to include definition everywhere as needed.

Redesign

That said, I think the proper solution would be to rethink your design and to use a container which will handle the destruction properly, so that you do not have to do it manually, which is tedious, and almost impossible to do correctly if you need exception safety. Use std::shared_ptr instead of raw pointers, or std::auto_ptr with a container which is able to hold them (std::vector cannot store auto_ptr values). One possible solution would be to use Boost Pointer Container

Suma
  • 33,181
  • 16
  • 123
  • 191
3

Is your clearPtrVector implementation in a header file? Because if it's in a separate .cpp file, the linker will not find it.

Igor Krivokon
  • 10,145
  • 1
  • 37
  • 41
  • Raindog you were right, that's what was happening in the first place. I forgot that it is not a class-owned function and had its implementation in a cpp file. Corrected that now .... and made a note to never do that mistake again :) . Thanks – Lefteris May 27 '09 at 08:08
3

A few things:

In your original code, don't pass in the size; just get it from the vector:

template <class S>
void clearPtrVector(std::vector<S*> &a)
{
    for(size_t i = 0; i < a.size(); ++i)
    {
         delete a[i];
    }

    a.clear();
}

Secondly, just pass in the vector itself, not the type it points to:

template <class Vector>
void clearPtrVector(Vector &vec)
{
    for(size_t i = 0; i < vec.size(); ++i)
    {
         delete vec[i];
    }

    vec.clear();
}

Thirdly, that error sounds like you have it placed in a .cpp file. The code will be generated when you first call the function, which means the compiler needs to know the definition of the function. Move the function into the header file, so the compiler can find it.

Lastly, consider using things more suited to this:

GManNickG
  • 494,350
  • 52
  • 494
  • 543
2

Make sure you have this function in a header file (.h, *.hpp) because if you defined it in a source file with a prototype in a header file you'll got an undefined reference linker error.

Undefined reference error means that compiler has found the reference of the function but linker has failed to find an reference of that function amongst object files. Any template function has to be defined in a header file so the compiler will be able to put it in any source file which makes use of the function.

Serge
  • 7,706
  • 5
  • 40
  • 46
0

Isn't the redesign a duplicate of cleaning-up-an-stl-list-vector-of-pointers

Remark

If you don't use one of the smart pointers, use boost::checked_delete function instead of delete to make sure you are not deleting an incomplete type.

Community
  • 1
  • 1
TimW
  • 8,351
  • 1
  • 29
  • 33
0

As I provided in my answer to your first question https://stackoverflow.com/questions/891913?sort=newest, one answer is as follows:

template <class C> void FreeClear( C & cntr ) {
    for ( typename C::iterator it = cntr.begin(); 
              it != cntr.end(); ++it ) {
        delete * it;
    }
    cntr.clear();
}

which works for all container types. Notice that the size parameter is NOT provided but is obrtained from the collection - this is the correct way to design such functions, providing the size as a separate value can only lead to disaster.

Community
  • 1
  • 1
  • Yeah I know you are right, I did it like that at first just out of bad habit. It is already corrected in my code. Thanks for taking the time to answer. What bothers me is so many people here telling me to use other kind of pointers which I don't even know how to use. Is it such a huge drawback in anyone's program to use normal pointers? – Lefteris May 27 '09 at 08:33
  • I use smart pointers (aout-ptr) in my code when I am constructing an object dynamically in stages (e.g. create object and then add children). I then hand off the autoptr contents to a collection of ordinary pointers, managed by a class object. I do this because I don't use boost & have issues with ref-counting re threading. –  May 27 '09 at 08:36
  • 1
    You shouldn't be bothered by people suggesting improved ways to program :) Learning them will only make you a stronger programmer. – GManNickG May 27 '09 at 08:39
  • I see. I guess I will have to check all my options and then settle on something that is correct and effective but also fits my programming habits (the good ones :p ) Thanks for the advice Neil. – Lefteris May 27 '09 at 08:40
  • 1
    Oh and GMan when I said bothered I meant ... not in the bad sense, just in the sense that makes you think over what you are currently doing :) – Lefteris May 27 '09 at 08:41