STL - Pointer to container Question

[gerard4143]

Could someone please verify. Is it proper to have pointers to containers(vectors in this example) where you allocate memory for them and then free them. I googled and found a posting that stated its considered poor coding style to do this...I doesn't seem like it should be poor programming style to me.

struct student contains the pointer

std::vector<double> *assignments;

Which I allocate in the program and then free towards the end with..

for (i = 0; i < the_students_size; ++i)
{
the_students.assignments->clear();
delete the_students.assignments;
}

#include <iostream>
#include <string>
#include <vector>
#include <algorithm>


//struct with pointer to vector....
struct student
{
  std::string name;
  double midterm, final;
  std::vector<double> *assignments;
};

bool student_compare(const struct student & lhs, const struct student & rhs)
{
  return lhs.name < rhs.name;
}

int main(int argc, char**argv)
{
  struct student a_student;
  std::vector<struct student> the_students;
  int choice;
  double x;
  
  while (true)
  {
    std::cout<<"(0)exit (1)set (2)sort and view->";
    std::cin>>choice;
    
    if (choice < 1) break;
    
    if (choice == 1)
    {
      a_student.assignments = new std::vector<double>;
      
      if (!a_student.assignments) return 1;
      
      std::cout<<"Please enter your name->";
      std::cin>>a_student.name;
      
      std::cout<<"enter midterm and final marks->";
      std::cin>>a_student.midterm>>a_student.final;
      
      std::cout<<"Please enter assignment marks "
		"Eof file for completion"<<std::endl;
      
      while (std::cin>>x)
	a_student.assignments->push_back(x);
      
      std::cin.clear();
      
      the_students.push_back(a_student);
    }
    
    if (choice == 2)
    {
        std::vector<struct student>::const_iterator begin_iter = the_students.begin();
	std::vector<struct student>::const_iterator end_iter = the_students.end();
	
	std::sort(the_students.begin(), the_students.end(), student_compare);
	
      while (begin_iter != end_iter)
      {
	std::cout<<"Name->"<<begin_iter->name<<std::endl;
	std::cout<<"Midterm->"<<begin_iter->midterm<<" Final->"<<begin_iter->final<<std::endl;
	
	std::vector<double>::const_iterator ibegin_iter = begin_iter->assignments->begin();
	std::vector<double>::const_iterator iend_iter = begin_iter->assignments->end();
	
	while (ibegin_iter != iend_iter)
	{
	  std::cout<<*ibegin_iter<<" ";
	  ++ibegin_iter;
	}
	std::cout<<std::endl;
	++begin_iter;
      }
    }
  }
  
  
  std::vector<struct student>::size_type the_students_size = the_students.size();
  std::vector<struct student>::size_type i = 0;
  
  //delete assigment vectors...
  for (i = 0; i < the_students_size; ++i)
  {
    the_students[i].assignments->clear();
    delete the_students[i].assignments;
  }
  the_students.clear();
  return 0;
}

[StuXYZ]

Introductory Note: You have not written std::vector<double*> S; . Which is a completely different discussion. This is often frowned on because it is SO easy to mess up, the memory [de]allocations.

Ok: In the general sense it is perfectly fine to have a point to a vector, or any other stl class. There are several "obvious" reasons for having a pointer, e.g.
(a) you could have extended a standard class and have a polymorphic system.
(b) you have a selection of classes and were avoiding a bit of memory/resource etc.
(c) You want to transfer ownership of the pointer between objects.

Of those I think (a) and (b) are not good reasons:
(a) isn't good, since the lack of virtual functions in the STL implementations don't make them easy classes to extend.
(b) The size of the class is minimal relative to a filled class. Normally has zero bearing on memory footprint etc.
(c) I am kind of ok with, but I would normally wrap it in another class, with better control and semantics.

Now let us look at what you are actually doing and see if I agree.

Well every student has a vector<double> assignment, so you are only interesting in option (c). Indeed this does make your sort quicker. However, the cost is the potential bugs that you will introduce.
However you can effectively have what you want (and slightly more optimization), by moving the pointer one level
higher. E.g. Making the it std::vector<student*> the_students; . That would actually give you
better optimization and slightly less book-keeping.

Personally, I prefer to put those pointers in a smart pointer like boost::shared_ptr. That way you get the optimization you are looking for, without all the bookkeeping requirement.

========
Secondary notes:
You don't need to clear vectors that are going out of scope, that happens anyway, you only have to delete the memory.

You test that the new returns a zero. This doesn't happen any more, new throws if it is unsuccessful.

you don't need to write everything like this std::vector<struct student> the_students; , (it is acceptable but very uncommon). You can drop the word: struct and write std::vector<student> the_students; . A struct is just a class, except that it starts in public rather than private.

Your naming of begin_iter and ibegin_iter confused me for some time , after you have been smart enough to avoid the using namespace std; trap, I expected highly distinguished variable names, which you have throughout the rest of your code.

======
Finally this is my opinion and not an issue of the standard, if this style fits your personality and project better than anything else, please use it. [unless you work for me, of course! :) ]

[gerard4143]

StuXYZ...Thanks for the reply and pointers, everything you stated makes sense except for the error on line 78. Are you sure about that one because I don't see it.

[StuXYZ]

Sorry, as I was reviewing my post, I then had a big edit of it... then I spotted the error....

Line 78 is fine, it was me miss reading the variable name. [Sorry for the confusion]

[gerard4143]

No problems...Again thanks for the pointers...