3
\$\begingroup\$

The wrapper makes it easy to manage memory when using CoreFoundation objects. It aims to act like a simple shared_ptr.

Here is a usage example:

cfobject_wrapper<CGImageSourceRef> imgSource(CGImageSourceCreateWithData(...), false);
...
dispatch_async(..., ^{
  foo(imgSource);
});
// no need to manually call CFRelease, and the CGImageSourceRef is automatically retained when copied to block and released when the block is destroyed.

Here is the code of the wrapper:

/** A wrapper for Apple's Core Foundation objects */
template<typename _T>
class cfobject_wrapper final
{
protected:
  _T _internal;

public:
  cfobject_wrapper()
  : _internal(nullptr)
  {
  }

public:
  explicit cfobject_wrapper(const _T& ref, bool retain = true)
  {
    if (ref && retain)
      CFRetain(ref);
    _internal = ref;
  }

public:
  cfobject_wrapper(const cfobject_wrapper<_T>& ref)
  : _internal(ref._internal)
  {
    if (_internal)
      CFRetain(_internal);
  }

public:
  cfobject_wrapper(cfobject_wrapper<_T>&& ref)
  {
    _internal = std::move(ref._internal);
    ref._internal = nullptr;
  }

public:
  cfobject_wrapper& operator=(cfobject_wrapper other)
  {
    std::swap(_internal, other._internal);
    return *this;
  }

public:
  ~cfobject_wrapper()
  {
    if (_internal)
    {
      CFRelease(_internal);
      _internal = nullptr;
    }
  }

public:
  _T internal() const
  {
    return _internal;
  }

public:
  explicit operator bool() const
  {
    return _internal != nullptr;
  }

};
\$\endgroup\$
  • 1
    \$\begingroup\$ Use Swift and you'll get managed Core Foundation objects for free! – Just kidding, this is a very nice question, welcome as active member on Code Review! \$\endgroup\$ – Martin R Oct 22 '17 at 20:28
  • \$\begingroup\$ I believe creating deleter class and making an alias to unique ptr with the type and the deleter would reduce the complexity a lot. \$\endgroup\$ – Incomputable Oct 22 '17 at 20:52
1
\$\begingroup\$

This is a really great idea! I could certainly use a class like this. Here are some thoughts:

Don't Multiply Declare public

It's very odd that you're prefixing every single method with its visibility. I really hate working with code that has multiple public, private or protected sections. I expect all public methods and members to be grouped together, all protected methods and members to be grouped together, and all private methods and members to be grouped together (usually in that order). While it's not required by the language, it's a pain to constantly have to figure out which section you're in when reading, and it's a pain to write it out for every method prototype when you're writing the code.

Avoid protected Member Variables

In general protected member variables are a bad idea. They essentially mean that any other object of the class or subclass can change their value out from under you. There's a case to be made for public member variables when performance is of the utmost importance, but in general member variables should be private so you can control who's changing them and when.

Retains and Releases Should Be Matched

It's a little surprising that I can tell the constructor not to retain the object, but the destructor will still release it. This is almost like a false cognate in another language - it looks similar to another pattern where you tell the constructor of some object that you're going to manually manage the memory for a given resource, so it shouldn't delete it when destructed. I understand the reason why - some CF methods return retained objects. Still, as someone potentially reading the code, it's a little unexpected. In Objective-C you could autorelease the value when calling the constructor and not worry about the retain, but there's no such mechanism for the C interface to CoreFoundation. Furthermore, the likelihood that I'll remember to pass false to the constructor when I can't remember to do the release manually is unlikely. For that reason, I recommend at least removing the default value. I'm also left wondering if there's some better way to handle this particular case. (I guess a first-class interface to CoreFoundation from C++ would be the ideal answer, but that's not going to happen unless you write it yourself.)

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks! (1) One advantage of not grouping methods by visibility is that the source control can keep track the code in case that the function visibility is changed in the future. (2) protected member: Good point, thanks! (3) Retains/releases matching: It's like __bridge_transfer vs. _bridge so both are needed, but removing the default value is a good idea. \$\endgroup\$ – Ben-Uri Oct 23 '17 at 6:19
1
\$\begingroup\$

Missing functionality.

I don't seem move assignment. Since you have move construction and copy assignment that seems a bit strange.

Also I don't see a swap function. This becomes really useful when you use you object with containers. Also it makes implementing the move semantics so much more obvious.

Exceptions

Normally you want to mark the move operators (and swap) as noexcept. This allows for certain optimizations when your object is used by the standard containers.

Identifiers

Prefer not to start identifiers with an underscore. The rules about this are complicated and even if you know them most people don't. In your case the identifier _T is reserved for use by the implementation.

https://stackoverflow.com/q/228783/14065

Protected

Avoid the use of protected. There is no real reason for it. Also in your case the class is marked final so you can't derive from it so the protected provides no extra visability.

Just mark all members private.

Usage

Usally I am not a fan of auto conversion. But fro wrapper classes it makes some sense. If you have this wrapper object and you want to use it in standard calls you currently need to call internal to get the value.

I find it easier to provide a conversion operator so I can use it directly.

operator _T () const
{
  return _internal;
}

Now you can use your object as if it was a _T.

using WrapImageSourceRef = cfobject_wrapper<CGImageSourceRef>;
WrapImageSourceRef imgSource(CGImageSourceCreateWithData(...), false);

CFStringRef ref = CGImageSourceGetType(imgSource);  // notice I don't need to call alything.
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Doesn't the copy assignment also implement the move assignment? stackoverflow.com/questions/36405412/… \$\endgroup\$ – Ben-Uri Oct 25 '17 at 12:53
  • \$\begingroup\$ @Ben-Uri: Why do you think that? Now it can be used by an R-Value reference. But it takes the R-Value reference and does a copy. Now since your object only contains a pointer the cost of a copy is low. But his becomes a maintenance issue as your class may be altered later and the editor may not notice you don't have an explicit move and thus you now have an expensive copy. \$\endgroup\$ – Martin York Oct 25 '17 at 17:08
  • \$\begingroup\$ Because the copy of the rvalue is elided: en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Copy-and-swap \$\endgroup\$ – Ben-Uri Oct 26 '17 at 19:22
  • \$\begingroup\$ @Ben-Uri: I am still a bit weary about it because of the typically elided comment. But I also think you are correct though, I need to check but, I believe I have heard that before. On a side note that page is full of some shit. A check for self assignment is not strictly necessary but can give some performance improvements \$\endgroup\$ – Martin York Oct 26 '17 at 19:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.