Please make ScopedPointer more complex


#1

It would be nice to extend ScopedPointer to include a policy object:

template <class ObjectType, class PolicyType = DefaultScopedPointerPolicy <ObjectType> >
class ScopedPointer
{
//...

Now we define the default policy:

template <class ObjectType>
struct DefaultScopedPointerPolicy
{
  void destroy (ObjectType* object) { delete object; }
  void assign (ObjectType* object) { }
  void release (ObjectType* object) { }
};

At this point, ReferenceCountedObjectPtr can reuse most of the code:

template <class ReferenceCountedObjectClass>
class ReferenceCountedObjectPtr : public ScopedPointer <ReferenceCountedObjectClass,
  ReferenceCountedObjectPolicy <ReferenceCountedObjectClass> >
{
public:
  //...
};

Implementing the policy object for ReferenceCountedObjectPtr is easy:

template <class ReferenceCountedObjectClass>
struct DefaultScopedPointerPolicy
{
  void destroy (ReferenceCountedObjectClass* object) { releaseObject (); }
  void assign (ReferenceCountedObjectClass* object) { object->incrementReferenceCount (); }
  void release (ReferenceCountedObjectClass* object) { object->decrementReferenceCount (); }
};

The only thing left to do is rewrite ScopedPointer member functions to use the policy instead of the old hard coded behavior. Here’s an example:

before

    ScopedPointer& operator= (ObjectType* const newObjectToTakePossessionOf)
    {
        if (object != newObjectToTakePossessionOf)
        {
            ObjectType* const oldObject = object;
            object = newObjectToTakePossessionOf;
            delete oldObject;
        }

        return *this;
    }

after

    ScopedPointer& operator= (ObjectType* const newObjectToTakePossessionOf)
    {
        if (object != newObjectToTakePossessionOf)
        {
            ObjectType* const oldObject = object;
            object = newObjectToTakePossessionOf;
            PolicyType::assign (object);
            PolicyType::release (oldObject);
        }

        return *this;
    }

Now we can delete most of the code in ReferenceCountedObjectPtr. Less code, and way more complicated!

But then someone could implement their own custom policy. For example, when the underlying object exists in an external DLL and we can’t just call operator delete (because there are two instances of the C Runtime heaps). This would make ScopedPointer more broad.


#2

So it turns out the policy only needs two functions: move() and reset().

template <class ObjectType>
struct AddonScopedPointerPolicy
{
  inline void reset (ObjectType* object, ObjectType** pDest)
  {
    if (*pDest != object)
    {
      if (*pDest != nullptr)
        *pDest->destroyObject ();

      *pDest = object;
    }
  }

  inline void move (ObjectType** pSrc, ObjectType** pDest)
  {
    if (*pSrc != *pDest)
    {
      if (*pDest != nullptr)
        *pDest->destroyObject ();
      *pDest = *pSrc;
      *pSrc = nullptr;
    }
  }
};

#3

With SFINAE (http://en.wikipedia.org/wiki/Substitution_failure_is_not_an_error) the policy members could be overloaded, and a single ScopedPointer object (maybe SmartPointer?) would be capable of holding both reference counted and non-reference counted objects with proper behavior, and no additional action required by the caller.

This means any existing container could be made to correctly hold reference counted objects, just by using ScopedPointer <ReferenceCountedObject > as the template parameter to the container.

No more ReferenceCountedArray or OwnedArray, a normal Array could hold reference counted or scoped objects transparently. And existing containers that are not reference count aware could hold reference counted objects without modification (SparseSet, SortedSet, etc).


#4

Dear god, no!! Awful!

The reason the class is good is because it’s so simple - it does one job very effectively. If you want to do a different job, use a different class, with a different name that explains what it does!


#5
class UniversalPointer

#6

Vinnie, it sounds like too much template metaprogramming is started to affect your mind: don’t be tempted down the path of over-generalisation!


#7

This all came up because I have moved some LGPL code into shared libraries / DLLs. I wrapped this code in a class interface. The shared library creates the object and returns it to the application. The problem comes when you want to delete the object. It’s not permissible to just call operator delete, since that memory for the object was allocated by the DLL (I’m using statically linked C runtime with delayed loading).

Therefore, the interface for deleting these objects from the host is a little different. Instead of calling operator delete, you call a member named destroyObject(). The class can be reference counted. Although I couldn’t use the JUCE reference counted object (you can’t override the delete behavior). It still has the same interface (incReferenceCount, decReferenceCount).

Unfortunately, neither ScopedPointer nor ReferenceCountedObjectPtr can be used with these objects. The reason being, that these containers are not sufficiently generic (nor can they be customized, due to the absence of a policy type).

The jobs are all the same - scoped lifetime management of a dynamically allocated object. The only difference is in the implementation detail. Why should the caller be required to use a different class depending on the implementation of the underlying object?

My original code used ScopedPointer and operator new, because I had not separated out the LGPL code yet. After I separated out the code, I still had the same class but now I can’t use a ScopedPointer anymore because of an implementation detail? That makes no sense.

If you think about containers like OwnedArray versus ReferenceCountedArray, its a similar situation. Every time you want to make a container ReferenceCountedObject-aware, you have to rewrite it? That’s madness!


#8

LOL we call that being an architecture astronaut

But seriously, this is a legitimate use-case. I lost the ScopedPointer functionality when I changed my class to come from an external DLL and I was not happy about losing the benefits of RAII patterns. So I had no choice but to write my own ScopedPointer to support this object. Why shouldn’t I have made it general / policy-based? Look how elegant it is:

// based on juce::ScopedPointer<>
template <class ObjectType,
          class PolicyType = AddonScopedPointerPolicy <ObjectType> >
class ScopedPointer
{
public:
  typedef ScopedPointer <ObjectType, PolicyType> ScopedPointerType;

  inline ScopedPointer() : m_object (nullptr)
  {
  }

  inline ScopedPointer (ObjectType* const object) : m_object (nullptr)
  {
    PolicyType::reset (object, &m_object);
  }

  ScopedPointer (ScopedPointer& other) : m_object (nullptr)
  {
    PolicyType::move (&other.m_object, &m_object);
  }

  inline ~ScopedPointer()
  {
    PolicyType::reset (nullptr, &m_object);
  }

  ScopedPointer& operator= (ScopedPointer& other)
  {
    PolicyType::move (&other.m_object, &m_object);

    return *this;
  }

  ScopedPointer& operator= (ObjectType* const object)
  {
    PolicyType::reset (object, &m_object);

    return *this;
  }

  inline operator ObjectType*() const
  {
    return m_object;
  }

  inline ObjectType* get() const
  {
    return m_object;
  }

  inline ObjectType& operator*() const
  {
    return *m_object;
  }

  inline ObjectType* operator->() const
  {
    return m_object;
  }

  ObjectType* release()
  {
    ObjectType* object = nullptr;

    PolicyType::move (&m_object, &object);

    return object;
  }

  void swapWith (ScopedPointerType& other)
  {
    ObjectType* temp = nullptr;

    PolicyType::move (&other.m_object, &m_temp);
    PolicyType::move (&m_object, &other.m_object);
    PolicyType::move (&temp, &m_object);
  }

private:
  //==============================================================================
  ObjectType* m_object;

  const ScopedPointer* getAddress() const
  {
    return this;
  }

#ifndef _MSC_VER
  ScopedPointer (const ScopedPointer&);
  ScopedPointer& operator= (const ScopedPointer&);
#endif
};

template <class ObjectType, class PolicyType>
bool operator== (const ScopedPointer <ObjectType, PolicyType>& pointer1, ObjectType* const pointer2)
{
  return static_cast <ObjectType*> (pointer1) == pointer2;
}

template <class ObjectType, class PolicyType>
bool operator!= (const ScopedPointer <ObjectType, PolicyType>& pointer1, ObjectType* const pointer2)
{
  return static_cast <ObjectType*> (pointer1) != pointer2;
}
#endif

This is my custom policy:

template <class ObjectType>
struct AddonScopedPointerPolicy
{
  static inline void reset (ObjectType* object, ObjectType** pDest)
  {
    if (*pDest != object)
    {
      if (*pDest != nullptr)
        (*pDest)->destroyObject ();

      *pDest = object;
    }
  }

  static inline void move (ObjectType** pSrc, ObjectType** pDest)
  {
    if (*pSrc != *pDest)
    {
      if (*pDest != nullptr)
        (*pDest)->destroyObject ();
      *pDest = *pSrc;
      *pSrc = nullptr;
    }
  }
};

#9

Certainly a need for something like this. I use boost’s shared_ptr custom Deleter when necessary… But a JUCEy solution would be nice…


#10

I quite like it too, tbh :slight_smile:


#11

These battles between Vinnie and Jules are tremendously entertaining :smiley:


#12

I have to say whilst Vinn makes a good case for the extended functionality one of the best things about the JUCE library is its simplicity and readability. Trying to tech someone RAII techniques is made very easy with classes like ScopedPointer. Adding somewhat bespoke functionality to a class like that massively complicates the issue. Doxygen also does a pretty bad job of displaying heavily templated classes. Maybe it should go into its own new class as suggested ‘UniversalScopedPointer’ or something more explicit.


#13

You’re right that the simplicity of JUCE is a great feature of course. But the API of ScopedPointer wouldn’t change, nor would the Doxygen comments. With my proposed improvement, ScopedPointer is still used in exactly the same way as before.

Did you look at the last two code snippets?


#14

Yeah, whilst I like it, it would certainly be best as a separate class (whether ScopedPointer and the like are replaced for typedefs is another discussion). I don’t think it’d make a huge amount of difference though; you still have to define your behaviour, and changing which type you want to use in an existing situation is going to involve changing the class type of any variables regardless. I guess it does take care of some boilerplate code in quite a neat way, though… but they’re not tremendously difficult classes to write, and either way, you’re following a specific interface.

Irrespective of all that, I’m definitely going to pinch it!


#15

I guess I wasn’t clear or I was assuming too much. It would be possible to make a default policy with specializations so that changing the type you want to use does NOT require a change (hence “Universal”), e.g.:

class MyObject;
class MyReferenceCountedObject : public ReferenceCountedObjectPtr;
class MyExternalLibraryObject : public MyCustomObject;

These would all work:

UniversalPointer <MyObject> p1;
UniversalPointer <MyReferenceCountedObject > p2;
UniversalPointer <MyExternalLibraryObject> p3;

Note in the examples above, the second template argument (policy) is omitted, since it uses the default. JUCE can provide two simple default policies, using template specialization.

First the generic specification:

template <class ObjectType>
class DefaultUniversalPointerPolicy
{
  // original ScopedPointer semantics

Now a specialization:

template<>
class DefaultUniversalPointerPolicy <ReferenceCountedObjectPtr>
{
  // policy to call incReferenceCount() and decReferenceCount()

At this point, UniversalPointer works with any type of standard object or ReferenceCountedObject.

Now, if you want to define a custom policy you can do so by specializing the default policy:

template<>
class DefaultUniversalPointerPolicy <MyCustomObject>
{
  // custom stuff

Given

class MyObject : public ReferenceCountedObject
{
};
UniversalPointer <MyObject> p;

You can change the base class of MyObject without changing any references in UniversalPointer:

class MyObject : public MyCustomObject
{
};
UniversalPointer <MyObject> p; // still works!

Now that’s cool!!!

All the existing functionality of ScopedPointer, no change in calling code, and supporting not only ReferenceCountedObjectPtr correctly but any user defined policy without changing any existing declarations! WOW!!!


#16

Seems like a whizzy idea, but ScopedPointer and ReferenceCountedObjectPtr are very different classes. The behaviour of their operator= and copy constructors is very different indeed, and attempting to cram two different behaviours into one class is a bad idea IMHO. For example, if you had a class that was reference counted, and had a program full of UniversalPointers to that class, then if you changed the class so that it’s no longer ref-counted (or vice-versa), then you’d be silently changing the behaviour of all your pointers, with possibly disastrous effects.

Besides, now that c++11 provides unique_ptr, my plan is that ScopedPtr will eventually become just a typedef for unique_ptr, because it uses new language features to do a better job with a few edge-cases.


#17

A valid point. Fortunately, anyone attempting to make such a change would find out pretty fast!! lol

Perhaps merging reference counting and scoped lifetime was overly ambitious, for the exact reason you pointed out. Still, having policy based pointer containers is useful. So…

class UniversalScopedPointer;
class UniversalReferenceCountedObjectPtr;

This would keep the behaviors separate but still offer policy based customizations. Fortunately, these classes can be implemented parallel to JUCE and coexist in peace and harmony.


#18

“Deleter”… seems they thought of everything