top | item 7612121

(no title)

michaelrmmiller | 12 years ago

True but that means you need a wrapper function for each type you want to create or you need to use the more general function, defeating the purpose.

It also obscures the type of the object. That can be okay if it's only used as a temporary local, but problematic if you ever need to store it as a member or return it from another function. The type signature is verbose and not very descriptive, either. Compare that to:

  namespace sdl2 {
    using window = std::unique_ptr<SDL_Window, deleter<decltype(SDL_DestroyWindow), SDL_DestroyWindow>>;
  }
  ...
  sdl2::window window{SDL_CreateWindow(...)};
If you wanted to do something like what they have, I would create a variant on C++14's std::make_unique. That could look like:

  template<typename T> struct deleter_traits;

  template<> struct deleter_traits<SDL_Window>
  {
    using custom_deleter = deleter<decltype(SDL_DestroyWindow), SDL_DestroyWindow>;
  }

  template<typename T, typename... Args> create_unique(Args&&... args)
  {
    return std::unique_ptr<T, typename deleter_traits<T>::custom_deleter>{new T(std::forward(args))};
  }

  sdl2::window window = create_unique<SDL_Window>(...);
That looks a lot cleaner to me, at least. And instead of having to write a new function for each type, you just add a new deleter_traits specialization.

(I haven't actually compiled any of this so it's probably riddled with syntax errors... hopefully the point still comes across!)

discuss

order

devcodex|12 years ago

I definitely like the cleanliness of this solution but I'm still not sure how to work this into the general solution yet. The purpose was to take an opportunity to learn about how to implement reusable library code (as well as test drive a few C++14 features).

That said, I do like this approach and it gives me more to consider when approaching a problem like this again. I'm writing to learn so I welcome the constructive criticism.

michaelrmmiller|12 years ago

Ah, good point! My code is pretty flawed... it's using new to allocate an SDL_Window! All of this is just aesthetics at a certain point and what you prefer. For me, I'd get bored writing make<SDL_Window, SDL_CreateWindow, SDL_DestroyWindow> over and over. One solution is to wrap that in a function make_window like you've done. The other would be to wrap it in a template function that uses traits to lookup the creator and deleter functions. Pretty much equivalent. I only slightly prefer the latter because it exposes the type of the object in the call.

Thanks for writing the article! I'm much more in this camp of writing thin resource management rather than full-blown wrappers. And even if I were writing a wrapper, I would use these same techniques you're exploring to manage the resource as a member. It removes the need for any boilerplate whatsoever and in one line expresses all you need to know about the semantics of the object.