Wrapper class for std::list that supports pointers and non-pointers

Is this getting philosophical when I start to theorize about deleting objects you don't really 'own'? And yes, i mean it.

Or, to be more blunt: You are deciding to use a container to store pointers. Fine, yet I'd rather let the coder/you to be responsible for the cleanup, not the container - and neither would I expect that - just think of a

List<char*> MyList;

  MyList.Add'("this is a string literal"); // OK, I am making the method's name up, but you know what I mean

  MyList.Clear(); // now you'dend up in calling 'delete' for a string literal -> instant segfault

Select allOpen in new window

Thanks for your response jkr,
though I did ask for helpful responses, not philosophical diatribes.

I am providing a template class for developers to use, as such, it is my responsibility to try and ensure that the code is as robust as possible.
I don't agree with your argument about "...deleting objects I don't really own". By providing a container for objects, or pointers to objects, that container is accepting responsibility!

If my class is not going to accept responsibility, then it is my responsibility as the designer to let the programmers using my class know in some way, either at compile time or run-time, that this is the case.

You could specialize the Clear function for pointers, so you can do something different for pointers compared to objects.

However, you should probably use smart pointers instead (ala boost::shared_ptr eg.), and avoid the whole issue altogether. While at the same time addressing the ownership issue jkr pointed out.

I hear what you are saying about smart pointers, and I have in fact created a template class that uses the smart_ptr<>.
However, I have created a non-pointer class for objects, and am trying to cater for the case of someone using it to store raw C++ pointers.

If My class is declared thus:
List<dummyObject*> myList
and I call myList.Clear()
all is well.

However, If my class is declared thus:
List<dummyObject> myList
and I call myList.Clear()
I get a compiler error at the line with the delete statement.

My current solution is to provide a second method called ClearPointers()
which will use RTTI to determine if the type in the list is a pointer, and if so, iterate through the list, popping and deleting, and if not, just calling the std::list::clear() method.

In the Clear() method, I use RTTI to test the type stored in the list, and if it is a pointer to spit out an error message and abort, or throw an exception.

I was hoping for a solution that would allow me to have one method named "Clear", even if I had to overload it, Hence my question here.

Yes, you are suggesting something I have already done.
I guess I could just abandon the idea of a list template for non-pointers and stay with my design of list using shared pointers.
I would not use Boost. I see no point in dragging in third-party libraries with all their attendant problems when I can use standard C++. My list uses the shared pointer from the tr1 namespace of the standard template library.

The only drawback is that such a list is using the heap to store its data rather than the stack. A large list of objects might have resource implications.

>> Yes, you are suggesting something I have already done.

Are you sure ? The current solution you described involved providing a function with a different name. It did not involve specializing the existing clear function.


>> I would not use Boost. I see no point in dragging in third-party libraries with all their attendant problems

boost::shared_ptr is an include-only library, so no extra runtime dependencies. It's also a very well-tested library - I've never had any problems with it.


>> My list uses the shared pointer from the tr1 namespace of the standard template library.

That's ok too. It's compatible with boost::shared_ptr (apart from the different namespace).
You might be interested to know that std::shared_ptr is in C++11.


>> The only drawback is that such a list is using the heap to store its data rather than the stack. A large list of objects might have resource implications.

I'm not following you here. How would a list of objects or raw pointers involve the stack ? If you mean you are storing pointers to stack allocated objects, then you most definitely should NOT call delete for those pointers.

std::list's contents are on the heap, not the stack (unless you are doing some funky stuff with memory allocators, or are working on an exotic platform).

Sorry I was unclear.
I have created two template classes that wrap the std::list template class.
The first one stored shared pointers and there was no need to do anything else except call the std::list::clear method, since the shared pointer would call the objects destructor the minute it went out of scope. Done and dusted.

The second list class was designed to store objects, without making them pointers, eg

class Porridge;

List<Porridge> myList

The std::list::clear method is fine here too, since the object's destructor is called.
However, I can see someone using my class to store pointers, eg

List<Porridge*> myList

In this case, the destructors will not be called when the list is cleared.
I tried, using RTTI, to deterrmine if the type of object stored in the list is a pointer, if not then just call the std::list::clear method.
If it was a pointer, then iterate through the list and delete each item after popping it from the list.
Sounded simple, and it works if the list contains pointers
List<Porridge*> myList
but will not compile if it does not:
List<Porridge> myList

An error message is generated, pointing to the line with the delete element statement:
"error C2541: 'delete' : cannot delete objects that are not pointers"

I am trying to find a solution, hence my discussion on this forum.

>> I am trying to find a solution, hence my discussion on this forum.

And I offered you a solution in the form of a specialization of the Clear function for pointers. For an example, see my earlier post http:#37719093.

It does exactly what you need. It allows for both List<Porridge> and List<Porridge*>, and both will be handled appropriately - ie. nothing special will be done for objects, but pointers will be deleted.

Have a closer look at the example code, and play around with it to see what it does.

I did look at your suggestion.
It does not compile. I get the error

error C3860: template argument list following class template name must list parameters in the order used in template parameter list

at the end of the function declaration:

template<class TYPE>
void List<TYPE*>::Clear(void)
{
}

>>though I did ask for helpful responses, not philosophical diatribes.

That's exactly my point. If someone provides me a container that automatically deletes dynamically allocated content, I'd rather not use it. Since you stated you are creating a wrapper for a std::list, you are in this case adding unexpected behaviour.

>> error C3860: template argument list following class template name must list parameters in the order used in template parameter list

You might have missed something vital from my example ... Can you show the exact code that caused that error message ?

Or alternatively, try this compilable example without changing anything (I just added a main and some output) :

#include <list>
#include <iostream>

template <typename T>
class ListBase {
  public :
    ListBase(void) { }
    virtual ~ListBase(void) { }

    // ...

    void clear(void);

  private :
    std::list<T> m_list;
};

template <typename T>
class List : public ListBase<T> {
};

template <typename T>
class List<T*> : public ListBase<T*> {
  public :
    void clear(void);
};

template <typename T>
void ListBase<T>::clear(void) {
  std::cout << "clear function for objects" << std::endl;
}

template<typename T>
void List<T*>::clear(void) {
  std::cout << "clear function for pointers" << std::endl;
}

int main(void) {
  List<int>  objList;
  List<int*> ptrList;
  objList.clear();
  ptrList.clear();
  return 0;
}

Select allOpen in new window

I get what you were driving at.
I renamed my List class as ListBase
 and then added an entry for
 
 template<class TYPE>
 class SList<TYPE*> : public SListBase<TYPE*>
 {
 public:
    inline void Clear()
    {
        if ( !m_list.empty() )
        {
            TYPE* item = m_list.front();
            while ( m_list.size() > 0 )
            {
                TYPE* item = m_list.front();
                delete item;
                m_list.pop_front();
            }
        }        
    }
 };
 
 and it worked fine.
 However, this solution broke something else.
 
 in my ListBase class I had defined two methods:
 template<class TYPE>
 class SListBase
 {
 public:
     typedef bool (*CompareFunction)( TYPE& lhs, TYPE& rhs );

...
    inline void Sort(void) { m_list.sort(); };
    inline void Sort( CompareFunction compFunc ) { m_list.sort( compFunc ); };
};

and I now get compiler errors:
Error      1      error C2784: 'bool std::operator <(const std::list<_Ty,_Ax> &,const std::list<_Ty,_Ax> &)' : could not deduce template argument for 'const std::list<_Ty,_Ax> &' from 'DummyClass'

If I comment out the code m_list.sort();
it compiles, but that is not a lot of use

>> Error      1      error C2784: 'bool std::operator <(const std::list<_Ty,_Ax> &,const std::list<_Ty,_Ax> &)' : could not deduce template argument for 'const std::list<_Ty,_Ax> &' from 'DummyClass'

Strange error. It seems to imply that you're trying to compare lists rather than the items in the list.

Could you post a complete compilable sample that has the same error ?

I have zipped up the Visual Studio 2008 project.
1. some of the files have an extension of "._txt" added to the end of their file extentions so that the File Attacher would work. Just remove the extension and it should all compile.
The line commented out in the MainApplication.cpp file
        //myList.Sort();   <<<<---- uncomment this line to see the problem
if uncommented, will illustrate the problem
ListPtrTesterForEE.zip

I don't see how that code implements the solution I proposed.

I also don't find a sort anywhere in that code.

That said, the only error I'm finding in the code, is that you're trying to instantiate a FindFunctor with a shared_ptr as third template parameter, whereas mem_fun_ref will expect that to be the object itself.

Something like boost::mem_fn (http://www.boost.org/doc/libs/release/libs/bind/mem_fn.html) supports smart pointers.

I am not surprised you don't recognise anything, as I sent the wrong project.
Note to self: Do nothing after midnight.

Here is the real project.

And to anyone who is kind enough to respond to this post: Nothing personal but I am NOT INTERRESTED in any solution that has the word "Boost" in it.

cheers

Maybe you could have a look at this one.
LinkTesterClearForEE.zip

>> Here is the real project.

It seems you haven't provided an operator< for DummyClass.

oops!
Forgot in all, the fuss about Clear().

One last question.
In the code I sent you I have defined an entry for Sort() in both the List<TYPE> and List<TYPE*> definitions. It now works thanks to your reminder about "<".
However, If I move the Sort() back to the ListBase() class I get the following error:

Error      1      error C2660: 'List<TYPE>::Sort' : function does not take 0 arguments

By my reasoning, the code should be able to see the definition in the base class because it is both a public method and is in a class that is publicly inhertied by both the List<TYPE> and List<TYPE*> classes.

Where is my reasoning flawed?

It should, yes. The original example code I posted did exactly that, but for the Clear function.

However, the error is because you've only moved the Sort without arguments back to the base class, and left the other overload in List<TYPE>.

The overload you left behind is hiding the Sort without arguments from the base class, so you cannot call it from a List<TYPE> object.

You can explicitly make the Sort without arguments visible again using :

using ListBase<TYPE>::Sort;

Select allOpen in new window

in the definition for the List<TYPE> class.

Excellent!
It works, just like a bought one!

Glad to hear that :)

This has been a learning exercise for me, both to expand my understanding of templates and of the behaviour of destructors.
The solutions here have proven valuable and workable.
However, I have found, despite the design to make the Clear() function work (which it now does), that if I just let the list of pointers fall out of scope, the destructors are not called. Even the std::list::erase( iterator ) method does not seem to call the destructor of the object, whether pointer or no.

I will abandon my design of the List class to incorporate pointers.
I will modify my List class to not accept pointer types, using RTTI to perform a check inside the List constructor and throw an exception if they try it.
I will recommend, in the notes in the header file, to use my List_Ptr class (using shared pointers) if they need to use pointers.
 I thank all for their contributions.

Sounds like a sane conclusion.

Except that I'd even simplify it further by keeping only one List class that can :

(a) store objects - which will be handled just fine, since their destructor will be called when they are removed from the list.

(b) store raw pointers - which will be stored of course, but NOT deleted when they are removed from the list. ie. the list has no ownership of the pointers, and the user of the list needs to be aware of that.

(c) store smart pointers - which will be handled just fine, since the smart pointers' destructor will be called when they are removed from the list.

Note that for none of these cases, anything special needs to be done. This is because the base std::list already does all that's necessary to handle these 3 cases as described.

What you might do though, is to document the fact that the list does not take ownership of raw pointers (which should be the expected behavior anyway).

Given that my shared pointer list is declared thus:

   template<class TYPE>
    class SList
    {
    public:
        typedef tr1::shared_ptr<TYPE> ItemPtr;
        typedef typename std::list<ItemPtr>::iterator ListIndex;
 
     public:
        SList_Ptr(void);
        SList_Ptr( const SList_Ptr& rhs );
        virtual ~SList_Ptr(void);

        inline void AddFirst( const ItemPtr& item );

        inline bool GetFirst( ItemPtr& item, ListIndex& pos );
        inline bool GetNext( ItemPtr& item, ListIndex& pos );

        ...
       
    private:  // member variables
        list<ItemPtr> m_list;
    };

    and my list or objects and pointers are declared as
             
template<class TYPE>
class ListBase
{
public:
    typedef typename std::list<TYPE>::iterator ListIndex;

 public:
    ListBase(void);
    ListBase( const ListBase& rhs );
    virtual ~ListBase(void);

    inline void AddLast( const TYPE& item );

    inline bool GetFirst( TYPE& item, ListIndex& pos );
    inline bool GetNext( TYPE& item, ListIndex& pos );

    inline void Reverse(void);

    inline void Sort(void);

    virtual void Clear(void) = 0;

private:  // methods
    void Copy( const ListBase& rhs );

protected:  // member variables
    list<TYPE> m_list;
};



// List of objects
template<class TYPE>
class List : public ListBase<TYPE>
{
public:
    using ListBase<TYPE>::Sort;

public:
    typedef bool (*CompareFunction)( TYPE& lhs, TYPE& rhs );
    inline void Sort( CompareFunction compFunc );
    inline void Clear();
};



// list of pointers to objects
template<class TYPE>
class List<TYPE*> : public ListBase<TYPE*>
{
public:
    typedef bool (*ComparePtrFunction)( TYPE* lhs, TYPE* rhs );
    inline void Sort( ComparePtrFunction compFunc );
    inline void Clear();
};

I can not quite see how to make a single declaration covering all your options.

>> I can not quite see how to make a single declaration covering all your options.

Like so :

template<typename T>
class List {
  public:
    // whatever functionality you need here ...

    void clear() { m_list.clear(); }

  private :
    std::list<T> m_list;
};

Select allOpen in new window

and then use it as you want :

List<int> intList;
// fill it ...
intList.clear();   // will work as expected

List<int*> rawPtrList;
// fill it ...
rawPtrList.clear();    // will remove the pointers from the list, but not delete them

List<shared_ptr<int> > smartPtrList;
// fill it ...
smartPtrList.clear();    // will work as expected

Select allOpen in new window

As I said, nothing special needs to be done. Just let the user decide which of these three options they need, and let them worry about the consequences of their choice.

It's a lot more flexible that way, and corresponds to people's expectance of how a container behaves.

You are going to hate this, but I found a solution to cleaning up a list of pointers when it just falls out of scope:

I added a virtual destructor to the List<TYPE> and List<TYPE*> classes, and just called Clear() inside the destructor.

The madness continues  :-)

Enjoy the madness :)