LinuxQuestions.org
Welcome to the most active Linux Forum on the web.
Home Forums Tutorials Articles Register
Go Back   LinuxQuestions.org > Forums > Non-*NIX Forums > Programming
User Name
Password
Programming This forum is for all programming questions.
The question does not have to be directly related to Linux and any language is fair game.

Notices


Reply
  Search this Thread
Old 09-20-2004, 03:52 AM   #1
R00ts
Member
 
Registered: Mar 2004
Location: Austin TX, USA
Distribution: Ubuntu 11.10, Fedora 16
Posts: 547

Rep: Reputation: 30
Red face C++ template smart pointers for class hiearchies in a vector push_back error


Ok, I need a little help here with an error I can't figure out. I have an abstract class that multiple subclasses inherit from. I contain the abstract class objects in a smart pointer. I have a vector of smart pointers to these class objects inside another class. Here's the code (the GameModeManager class is the important part):

Code:
/******************************************************************************
 * SmartPtr class - A template class for creating smart pointers
 *
 *  members: T* ptr: the pointer to the object that has been wrapped
 *
 *****************************************************************************/
template<class T> class SmartPtr {
  T* ptr;
public:
  explicit SmartPtr( T* p = 0 ) : ptr(p) {}; // Gives a default value of 0 to ptr.
  // 'explicit' prevents the use of implicit type conversion in the constuctor  
  ~SmartPtr() { delete ptr; }
  
  T& operator*() const { return *ptr; }
  T* operator->() const { return ptr; }
  
  // The release and reset functions are used for the copy constructors.
  
  T* release() { // Saves the value of ptr, sets it to null, and then returns the old pointer
    T* old_ptr = ptr;
    ptr = 0;
    return old_ptr;
  }
  
  void reset( T* new_ptr ) { // Destroys the old pointer and assigns the new value
    if ( ptr != new_ptr ) {
      delete ptr;
      ptr = new_ptr;
    }
  }
  
  // Copy constructor
  SmartPtr( SmartPtr<T>& other ) : ptr( other.release() ) {}
  
  // Copy assignment operator
  SmartPtr operator=( SmartPtr<T>& other ) {
    if ( this != &other )
      reset( other.release() );
    return *this;
  }
  
};

/******************************************************************************
 * GameMode class - A parent class that all other game mode classes inherit from.
 *
 *  members: 
 *
 *  functions: Update():
 *               A purely virtual function that updates the game status
 *             Render():
 *               A purely virtual function that draws the next frame
 *
 *  notes:
 *****************************************************************************/
class GameMode {
public:
  virtual void Update() = 0;
  virtual void Render() = 0;
};


class BootMode : public GameMode {
public:
  void Update() { cout << "BootMode Update()" << endl; }
  void Render() { cout << "BootMode Render()" << endl; }
  ~BootMode() { cout << "BootMode Destructor" << endl; }
};

class BattleMode : public GameMode {
public:
  void Update() { cout << "BattleMode Update()" << endl; }
  void Render() { cout << "BattleMode Render()" << endl; }
  ~BattleMode() { cout << "BattleMode Destructor" << endl; }
};

class MapMode : public GameMode {
public:
  void Update() { cout << "MapMode Update()" << endl; }
  void Render() { cout << "MapMode Render()" << endl; }
  ~MapMode() { cout << "MapMode Destructor" << endl; }
};

/******************************************************************************
 * GameModeManager class - Contains a stack of GameMode objects for organizing recent game modes.
 *
 *  members: vector<GameMode *> mode_stack: a stack containing all of the current game modes.
 *
 *  functions: Pop():
 *               Removes the top item from the stack and destroys it.
 *             Push():
 *               Puts a new item on the top of the stack.
 *             Update():
 *               Calls the Update function for the GameMode object on the top of the stack
 *             Render():
 *               Calls the Render function for the GameMode object on the top of the stack
 *
 *  notes: 1) The stack should never be empty(?).
 *****************************************************************************/
class GameModeManager {
   vector< SmartPtr<GameMode> > stack;
   int top;
public:
  void Pop() { 
     stack.pop_back();
     if (top != 0)
       top--;
  }
  void Push(SmartPtr<GameMode> gm) { 
//    stack.push_back(gm);
    top++;
  }
   void Update() { (*stack[top]).Update(); }
   void Render() { (*stack[top]).Render(); }
I can't seem to figure out the error inside GameModeManager.Push() where I try to push the new SmartPtr object onto the stack. I would post the compiler error here, but its more than a page and its just a big old mess. I tried removing the <GameMode> in the function argument and I got an even longer error. Can anyone see what's wrong? And also if you see something else bad with my code tell me. I usually don't code well at 4AM.
 
Old 09-20-2004, 04:02 AM   #2
rjlee
Senior Member
 
Registered: Jul 2004
Distribution: Ubuntu 7.04
Posts: 1,994

Rep: Reputation: 76
Edit: This post may be wrong. I had thought that delete()ing a null pointer was an error but having checked, it seems I was wrong. Thanks to dakensta for pointing this out.

In the SmartPtr destructor, you should test to see if ptr is NULL before deleting it. Attempting to delete a NULL pointer is an error in C++ (although some compilers like MSVC++ treat it as a no-op but the standard doesn't).

This could possibly be causing the problem, as the pointer is being passed by value and thus ptr is being set to NULL in the release() method of the copy constructor.

Last edited by rjlee; 09-20-2004 at 06:17 AM.
 
Old 09-20-2004, 05:38 AM   #3
dakensta
Member
 
Registered: Jun 2003
Location: SEUK
Distribution: Debian & OS X
Posts: 194

Rep: Reputation: 35
You don't have a proper copy contructor for SmartPtr ... you need to pass a const reference otherwise your class doesn't meet the requirements for use with a standard container where your objects may be copied though a temporary.

example:
vector:: push_back(const T& t) will copy 't' into the last postion in the vector.
However, your copy ctor uses a non-const reference.

Passing a const reference as a non-const reference is an error - you are trying to change something already declared constant.

Your problem is that you are not sharing ownership from one pointer to another, so really you need to implement some form of reference counting or deep copying to effectively use your SmartPtr in a standard container. Currently you are passing ownership, as with std::auto_ptr.

My general advice though : don't roll your own smart pointers (or stack for that matter)

As you are using Debian, apt-get libboost-dev (and -doc, I think) which will install it all for you and use one of their smart pointers or visit http://www.boost.org/ and have a look at their shared_ptr (under memory link).

You might want to consider a std::stack also (you can use whatever underlying container you want) rather than rolling your own.

Quote:
Attempting to delete a NULL pointer is an error in C++
http://www.parashift.com/c++-faq-lit....html#faq-16.7 or has therer been a recent change?

(edit) Give GameMode a virtual destructor.

Last edited by dakensta; 09-20-2004 at 05:59 AM.
 
Old 09-20-2004, 10:10 AM   #4
R00ts
Member
 
Registered: Mar 2004
Location: Austin TX, USA
Distribution: Ubuntu 11.10, Fedora 16
Posts: 547

Original Poster
Rep: Reputation: 30
Quote:
Originally posted by dakensta
[B]You don't have a proper copy contructor for SmartPtr ... you need to pass a const reference otherwise your class doesn't meet the requirements for use with a standard container where your objects may be copied though a temporary.

example:
vector:: push_back(const T& t) will copy 't' into the last postion in the vector.
However, your copy ctor uses a non-const reference.

Passing a const reference as a non-const reference is an error - you are trying to change something already declared constant.

Your problem is that you are not sharing ownership from one pointer to another, so really you need to implement some form of reference counting or deep copying to effectively use your SmartPtr in a standard container. Currently you are passing ownership, as with std::auto_ptr.
Thanks. I made the copy constructor and copy assignment operator pass const references and I added a virtual destructor for GameMode. I'm still a little confused though. So I can not have unshared ownership if I want this to work in a standard container? That kind of sucks but I can get around it. As for using that library you mentioned, I'm trying to minimize the number of dependencies for the application this will be used in so I'd really rather make my own smart pointer class than force my user to install a library. And the vector "stack" is just a temporary thing really. I'm still not sure if I will need to access elements that are not on the top of the stack or not so that's why its a vector.
 
Old 09-20-2004, 11:00 AM   #5
mhearn
LQ Guru
 
Registered: Nov 2002
Location: Durham, England
Distribution: Fedora Core 4
Posts: 1,565

Rep: Reputation: 57
You can always distribute your own static binaries, you know. That's what I'm going to do for my C++ app.
 
Old 09-20-2004, 11:49 AM   #6
R00ts
Member
 
Registered: Mar 2004
Location: Austin TX, USA
Distribution: Ubuntu 11.10, Fedora 16
Posts: 547

Original Poster
Rep: Reputation: 30
Quote:
Originally posted by mhearn
You can always distribute your own static binaries, you know. That's what I'm going to do for my C++ app.
I know. But I still want to give users the option to compile from source and I don't want it to be a big headache for them.
 
Old 09-20-2004, 12:35 PM   #7
dakensta
Member
 
Registered: Jun 2003
Location: SEUK
Distribution: Debian & OS X
Posts: 194

Rep: Reputation: 35
The requirements for an object in a container is that it is copy constructible.

The implication of something being copy constructible is that the two objects (constructed one and the one you copied from) are equivalent.

When you are copying an unshared smart pointer, after copying one has a valid pointer and one has a null pointer, so it breaks this equivalency.

If you can work around this, then feel free to use the standard containers.

Putting limitations on the types used is part of the price to pay for using templates - I can't really comment on the exact implications of not having this limitation, it is pretty low level in the standard library and I expect makes robust implementation very difficult (that's a guess!)

The boost libraries are often thought of as c++ in waiting ... parts of it will hopefully become standard at the next revision and many, if not most, of the people working on it are on or alfiliated with the c++ standards comittee.

In addition, a smart pointer could be a real killer if you find a bug in it later on ... there is a lot to be said for using something tried and tested:

Code:
  SmartPtr operator=( SmartPtr<T>& other ) {
    if ( this != &other )
      reset( other.release() );
    return *this;
  }
If I write this:
Code:
  SmartPtr<GameMode> p(new BattleMode);
  SmartPtr<GameMode> q;
  q = p;
Now what happens to q?
And the pointer from p?
In your assignment operator you have returned by value ...
...calling your 'copy constructor', which in turn has called other.release() and ... OUCH!
q.ptr is now 0

I sympathise with the issues of different libraries - that's just c++ for you But if there was one external library to use, I would take boost and most of the library is actually just header files so it is pretty easy to use and as such doesn't need to be included in a distribution (other than source code!)

Last edited by dakensta; 09-20-2004 at 12:41 PM.
 
Old 09-20-2004, 01:14 PM   #8
R00ts
Member
 
Registered: Mar 2004
Location: Austin TX, USA
Distribution: Ubuntu 11.10, Fedora 16
Posts: 547

Original Poster
Rep: Reputation: 30
Thanks that makes perfect sense to me now. I think I'll just do a work-around for this. I'll take a look at the boost library as well and see if there's anything else useful for me there.
 
  


Reply



Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is Off
HTML code is Off



Similar Threads
Thread Thread Starter Forum Replies Last Post
Implementing a vector class from a list class purefan Programming 9 04-14-2005 10:48 PM
problem with vector and push_back call kris55 Linux - Software 2 03-24-2005 08:10 PM
Template class with a template member... Nicholas Bishop Programming 3 02-21-2005 08:27 PM
template class ckcheung0927 Programming 3 11-28-2004 03:59 PM
Vector(STL) question(push_back & erase) Ohmu Programming 1 01-29-2004 04:50 PM

LinuxQuestions.org > Forums > Non-*NIX Forums > Programming

All times are GMT -5. The time now is 07:17 PM.

Main Menu
Advertisement
My LQ
Write for LQ
LinuxQuestions.org is looking for people interested in writing Editorials, Articles, Reviews, and more. If you'd like to contribute content, let us know.
Main Menu
Syndicate
RSS1  Latest Threads
RSS1  LQ News
Twitter: @linuxquestions
Open Source Consulting | Domain Registration