To PIMPL or not To PIMPL

Started by
18 comments, last by GenuineXP 14 years, 3 months ago
For what it's worth, if anyone is still listening:
class foo{    public:    typedef boost::shared_ptr<foo> ptr;};//now use foo::ptr
I use this in almost all my classes that use shared_ptrs. Keeps everything nice and wrapped.
Advertisement
That was another design I was considering Ocelot, but decided against it because it's not conducive to using forward declaration of unspecified types. The external typedefs allow me to use foo_ptr everywhere, rather than having to use shared_ptr<foo> anywhere the type isn't fully known. I think it makes the public interface a LOT more consistent and straightforward.
Quote:Original post by Shinkage
to say un-C++-like is a pretty silly, considering the language is fundamentally designed to allow for pretty much any syntax you want.
Taken to the extreme, that line of reasoning could be dangerous ;-D
Quote:Original post by Hodgman
Taken to the extreme, that line of reasoning could be dangerous ;-D


Well, as could any line of reasoning. This is why I think that it's absolutely stupid to talk about "never" and "always" in terms of programming design and techniques (with the obvious exception of things that don't work; a sensibility that the C++ FAQ linked earlier echoes). ALL that matters is how you use it, and whether the way you're using it is sensible, straightforward and effective.

And that's exactly what I was posting here to ascertain; whether other programmers thought it worked. And I agree with the conclusion that it does not.

EDIT: Ah, macros. I tend to only use them for two things: conditional compilation and preprocessor recursion. Other than that they really get in the way. I mean, whatever was Microsoft thinking defining macros called "min" and "max"...
I quite like pimpls myself - Use them to give global access to a subsystem whilst being able to change the subsystem without any other piece of code needing to know. Not really gonna happen much in a game but I used this feature of them in my dissertation for a live demo. I decided to template off my own one:

///////////////////////////////////////////////////////////////////////////** \class BcPimpl*	\brief Templated Pimpl design pattern.**	Like a singleton, but provides a global access point where*	there are potentially going to be multiple implementations.*	@param _Ty Class type for implementation.*/template < class _Ty >class BcPimpl{public:	BcPimpl();	~BcPimpl();	/**	*	Set the implementation. Will destroy previous if	*	there is one currently set.	*	@param pImpl Pointer to implementation	*/	static void pImpl( _Ty* pImpl );	/**	*	Get the implementation.	*	@return Poniter to implementation.	*/	static _Ty* pImpl();	/**	*	Destroy the implementation.	*/	static void pImplDestroy();private:	static _Ty* pImpl_;};//////////////////////////////////////////////////////////////////////////// Ctortemplate < class _Ty >BcPimpl< _Ty >::BcPimpl(){	}//////////////////////////////////////////////////////////////////////////// Dtortemplate < class _Ty >BcPimpl< _Ty >::~BcPimpl(){	BcAssert( pImpl_ == NULL );}//////////////////////////////////////////////////////////////////////////// pImpl//statictemplate < class _Ty >void BcPimpl< _Ty >::pImpl( _Ty* pImpl ){	if( pImpl_ != NULL )	{		delete pImpl_;	}	pImpl_ = pImpl;}//////////////////////////////////////////////////////////////////////////// pImpl//statictemplate < class _Ty >_Ty* BcPimpl< _Ty >::pImpl(){	return pImpl_;}//////////////////////////////////////////////////////////////////////////// destroy//statictemplate < class _Ty >void BcPimpl< _Ty >::pImplDestroy(){	delete pImpl_;	pImpl_ = NULL;}//////////////////////////////////////////////////////////////////////////// pImpl_template < class _Ty >_Ty* BcPimpl< _Ty >::pImpl_ = NULL;


And how I use it (could probably be refined at this point)

class CsCore:	public BcPimpl< CsCoreImpl >{public:	static void create();private:};


CsCoreImpl being the interface which must be specified, and the CsCore::create() function used to create whatever implementation of CsCoreImpl is required. A better example would be the renderer - create taking a parameter for which API to instanciate an implementation of.

I settled on this for all my subsystems after I went with singletons for one project, which I now despise - the idea of lazily creating a class on first instance doesn't sound pleasant to me, plus I wanted the benefit of being able to switch at run time rather than compile time.

I also purposely omitted the wrapping of all the calls - I felt it added an extra level of complexity (and a lot more pointless code written) that I didn't require in my case since "CsCore::pImpl()->doSomething()" is not a lot to type - the system class names are purposely kept short for that reason.
Adventures of a Pro & Hobby Games Programmer - http://neilo-gd.blogspot.com/Twitter - http://twitter.com/neilogd
Quote:Original post by Richy2k
I quite like pimpls myself - Use them to give global access to a subsystem whilst being able to change the subsystem without any other piece of code needing to know. Not really gonna happen much in a game but I used this feature of them in my dissertation for a live demo. I decided to template off my own one:


Um, I may be missing something, but what you have here doesn't seem to actually be a PIMPL pattern, since you're exposing it publicly. In fact, it seems like what you have here is a singleton by another name.

EDIT: Singletons don't have to use lazy construction; that's just one popular way of implementing them. I use a singleton more along the lines of this (off the top of my head):

template<typename T>class singleton{private:	static T *_instance;protected:	singleton()	{		if(_instance)			delete _instance;		_instance = (T*)this;	}		virtual ~singleton() { _instance = 0; }public:	static T* get_instance() { return _instance; }};


Actually, the exact design I use is a little more complex, but you get the idea: this singleton design simply makes sure you have one of something and gives global access to it. Whatever that something is still gets allocated in the normal way and becomes part of the program scope somewhere.
Quote:Original post by Shinkage
Um, I may be missing something, but what you have here doesn't seem to actually be a PIMPL pattern, since you're exposing it publicly. In fact, it seems like what you have here is a singleton by another name.


Depends what book you read - but I don't disagree with you. It is more of a singleton implementation than a pimpl as you can't have multiple instances of the same type of class. Rather than wrap the calls (why wrap? wasted programming time), I just decided to expose a pointer to the interface class - NOT the actual implementation. So I suppose if you want to get technical on naming, it's more of a Pinterface [smile]
Adventures of a Pro & Hobby Games Programmer - http://neilo-gd.blogspot.com/Twitter - http://twitter.com/neilogd
If you actually do want the class to be treated like a value type, then the underlying data should be copied when an instance is copied, which makes shared_ptr the wrong type of smart pointer. Consider, for example, the_edd's value_ptr, which was pretty much made for this sort of thing. :) Although shared_ptr is appropriate in cases where the pointed-at data is large and conceptually immutable under the public interface (e.g. if it points to a bitmap resource for a Sprite class).
Quote:Original post by Zahlman
If you actually do want the class to be treated like a value type, then the underlying data should be copied when an instance is copied, which makes shared_ptr the wrong type of smart pointer. Consider, for example, the_edd's value_ptr, which was pretty much made for this sort of thing. :) Although shared_ptr is appropriate in cases where the pointed-at data is large and conceptually immutable under the public interface (e.g. if it points to a bitmap resource for a Sprite class).


I in fact do NOT want this, which I think people picked up on and why it became clearly early that the proposed idea was a non-starter. Everything wrapped in a shared_ptr is a high level "management" type class. As an example, see the last code example at the bottom of this page.

(Disclaimer: The code on that page is just representative; the exact interfaces have already changed.)
Interesting, I was just wrestling with the same issue in some of my code. I realized since I was maintaining the implementations using boost::shared_ptr objects that the so-called implementation hosts could be used as value types. However, this lead to some confusing syntax (IMHO) and the problem that any data not stored in the implementation but rather in the host object would lead to unnecessary copying (in my case, copying shared signal objects).

I too have decided this isn't very desirable. :-) I have also adopted a simple class-level typedef for pointer types as mentioned previously in this thread.

class graphics_context : ... {...public:typedef boost::shared_ptr<graphics_context> ptr;typedef boost::shared_ptr<const graphics_context> const_ptr;...};

I think this will be much more obvious to users. There's also the problem of deferring instantiation, which would not be possible using value semantics.

This topic is closed to new replies.

Advertisement