Failing To Remove Last Vector Element

[zb0S216C]

#include <iostream>
#include <vector>

using std::vector;
using std::cin;
using std::cout;
using std::endl;

vector < int * > Vec;

void Init( const int Count )
{
      for( int i( 0 ); i < Count; i++ )
      {
            Vec.push_back( NULL );
            if( ( Vec.at( i ) = new int( 0 ) ) == NULL )
                  break;
      }
}

int main( )
{
      Init( 2 );

      for( unsigned int i( 0 ); i < Vec.size( ); i++ )
      {
            if( Vec.at( i ) )
            {
                  delete Vec.at( i );
                  Vec.at( i ) = NULL;
                  Vec.erase( Vec.begin( ) );
            }
            else
                  Vec.erase( Vec.begin( ) );
      }

      cin.get( );
      return 0;
}

When running this code, the last element of vec is never released. I've got a suspicion that it's Vec.erase( Vec.begin( ) ) on line 32. Can any find what I've done wrong?

[Athar]

This doesn't seem to make much sense. You delete the i'th element and then erase the first? Why?
Moreover, you're skipping the new i'th element after each erase, since you erased the first element (thus changing the indexes of all other elements) but aren't accommodating for this.

Other things: new cannot return NULL, so the check in line 16 has no effect.
Nor do you have to check a pointer for NULL when you delete it.

That gets us to:

for (uint i=0;i<vec.size();i++)delete vec[i];
vec.clear();

C++0x ways:

for (auto it=vec.begin();it!=vec.end();++it)delete *it;
for (int* p : vec)delete p;

Only true way:

#include <boost/ptr_container/ptr_vector.hpp>
boost::ptr_vector<int*> vec;
Init(2);
//no manual cleanup necessary 

[zb0S216C]

Thanks for the reply, Athar. I've changed the code to this:

#include <iostream>
#include <vector>

using std::vector;
using std::cin;
using std::cout;
using std::endl;

vector < int * > Vec;

int main( )
{
      Vec.push_back( new int( 10 ) );
      Vec.push_back( new int( 10 ) );

      int Working( 0 );

      while( !Vec.empty( ) )
      {
            Working = ( Vec.size( ) - 1 );
            if( Vec.at( Working ) )
            {
                  delete Vec.at( Working );
                  Vec.at( Working ) = NULL;
                  Vec.pop_back( );
            }
            else
                Vec.pop_back( );
      }

      cin.get( );
      return 0;
}

This code works now :)

Thanks again, Athar.

[Athar]

You're still doing it. Using that approach, it should be:

  while(!Vec.empty())
  {
    delete Vec.back();
    Vec.pop_back();
  }

[zb0S216C]

Lol. I thought I had it. In my defense, I'm still learning about vector. This is my new code:

#include <iostream>
#include <vector>

using std::vector;
using std::cin;
using std::cout;
using std::endl;

vector < int * > Vec;

int main( )
{
      Vec.push_back( new int( 10 ) );
      Vec.push_back( new int( 10 ) );

      while( !Vec.empty( ) )
      {
            if( Vec.back( ) )
            {
                  delete Vec.back( );
                  Vec.back( ) = NULL;
                  Vec.pop_back( );
            }
            else
                Vec.pop_back( );
      }

      cin.get( );
      return 0;
}

Have I done anything wrong?

[Disch]

this isn't wrong, but it's rather pointless:

1) no need to set it to NULL if you're going to be popping it right away
2) no need to check for NULL before deleting. The delete operator already does that (it's OK to delete a null pointer -- delete will just do nothing if that happens.

So...

      while( !Vec.empty( ) )
      {
            delete Vec.back();
            Vec.pop_back();
      }

[zb0S216C]

Well, you learn something new everyday.

Thanks, Disch, and Athar.