6 ms·
C++14 and SDL2: Managing Resources
- simias 13y agoWouldn't it be simpler to just wrap the SDL_Window C object in a C++ class? That seems like a lot of boilerplate to just save oneself a destructor call. Also, I'm not usually one to bash C++ for its syntax (I've written quite a lot of C++ in my days and still use it from time to time) but my god I find that template spaghetti code physically painful to read.
- plorkyeran 13y agoThe point of this is you can write it once and use it for every type rather than having to write a wrapper for each one.
- 10098 13y agoThis, and also it would bother me a lot to know that I'm essentially re-implementing what is already available in the standard library.
- simias 13y agoIf I understand correctly you still have to write something like template<typename... Arguments> auto make_window(Arguments&&... args) { return detail::make_resource(SDL_CreateWindow, SDL_DestroyWindow, std::forward<Arguments>(args)...); } for each resource. Quite honestly at that point I'd sooner use a good old variadic macro, C99 style. I mean, sure, it's arguably less "elegant" and C++-ish but it'll be easier to understand, to maintain and won't spam me with pages of arcane template errors when I forget a colon somewhere, so it'll be easier to debug as well. That's the double edged sword of the C++ template system I suppose, in order to gain in purity they lost the simplicity of the C macro system without gaining the expressiveness and convenience of the Lisp macro system. In the end, what percentage of C++ coders use templates outside of the STL you think?
- pja 13y agoThe C++ template will give you extra type checking that the varadic macro won't, which is worth the tradeoff for the obscure error messages for me. (At least the errors have got slightly better in recent g++ releases with colourised error output.)
- masklinn 13y ago> If I understand correctly you still have to write something like You don't have to, you can create further helper function calling the generic thing but it's optional.
- devcodex 13y agoThe code that makes use of templates and such is meant to be library code in the same way that STL is, not something the average user interacts with other than to know they need to pass an Init and Destroy function followed by initialization parameters. I don't see people tearing apart <tuple>'s code because of its complexity, or <vector> or any other reusable STL component that most developers who've complained about the verbosity of my solution use every day without batting an eye. In these proposed solutions I've not yet seen one that has the capability to do this: auto window = make_resource(SDL_CreateWindow, SDL_DestroyWindow, ....); With my solution all you have to do is call make resource with different parameters it automatically works with other C style init/destroy created resources, not just from the SDL library but any C library that follows that practice. Can you try posting what your solution to the general problem would look like?
- angersock 13y agoThe thing is, you're going to be wrapping in a class anyways, so why not move all the nonsense into a normal destructor and then use normal smart pointers? I suggest that this is somewhat solving the wrong problem.
- forrestthewoods 13y agoMy god. That's a hideous amount of hard to read code to support such a simple operation. Do not like. At all. (And I write C++ video game code all day every day)
- 10098 13y agoMaybe this one is simpler: http://onlinehut.org/2013/01/smart-pointers-are-not-just-for-memory/ http://onlinehut.org/2013/01/smart-pointers-are-not-just-for... though I was trying to use that method to manage handles, not pointers. I eventually had to write a wrapper for handles for it to fulfill the requirements imposed by unique_ptr.
- azth 13y agoI suppose you simply wrap the functionality in a class with a destructor? That's what I thought at first too; however, the author mentions in the comments section on that page something about this interacting in a hard fashion with C++11's rvalue references, and copy and move constructors. Another use on here suggested a shorter and more efficient way to write the code [0]. https://news.ycombinator.com/item?id=7610571 https://news.ycombinator.com/item?id=7610571
- angersock 13y agoYeah, honestly, I'd prefer our smart pointer class over that nonsense any day. You end up exposing the SDL functionality as a normal class object, and everything's much more straightforward.
- nanofortnight 13y agoThis is meant as library code, not application code, though for template work, this is actually quite lightweight. You'd see scarier creatures than this if you write C++ libraries. Perhaps you should take a look at the implementation of your standard libraries or your favourite boost library sometime. Let's break it down: If this weren't library code you mightn't care about proper forwarding (which isn't important anyway since the function you're calling is written in C and C doesn't have lvalue-references!! - unnecessary complexity), so you would sloppily write this as: template<typename Creator, typename Destructor, typename... Arguments> auto make_resource(Creator c, Destructor d, Arguments&&... args) { typedef decltype(*c(args...)) ResourceType; return std::unique_ptr<ResourceType, void(*)(ResourceType*)>(c(args...), d); } Which is much easier to understand, yes? Your standard passing multiple arguments along thing like you'd normally do when writing a custom printf. Actually, now that I've had time to think about it, you can simpify the original code: template<typename Creator, typename Destructor, typename... Arguments> auto make_resource(Creator c, Destructor d, Arguments&&... args) { auto r = c(std::forward<Arguments>(args...)); typedef typename std::decay<decltype(*r)>::type ResourceType; return std::unique_ptr<ResourceType, void(*)(ResourceType*)>(r, d); } And again, written sloppily: template<typename Creator, typename Destructor, typename... Arguments> auto make_resource(Creator c, Destructor d, Arguments&&... args) { auto r = c(args...); typedef decltype(*r) ResourceType; return std::unique_ptr<ResourceType, void(*)(ResourceType*)>(r, d); } Or, if you don't mind repetition of ResourceType: template<typename Creator, typename Destructor, typename... Arguments> auto make_resource(Creator c, Destructor d, Arguments&&... args) { auto r = c(args...); return std::unique_ptr<decltype(*r), void(*)(decltype(r))>(r, d); } Clearer?
- aktau 13y agoI can barely imagine a more fitting love sonnet for good old (modern) C99/11. My eyes, dear $deity my eyes. I'm sure OP is a fantastic(ally) smart dev but I'm kind of glad I don't have to work with that.
- mattgreenrocks 13y agoExcept it's not common code at all. Want to look at some tasteful C++ code? Check out LLVM.
- dfbrown 13y agoThis is no worse than anything else in the standard library. Writing generic template code is always ugly but once written you don't have to deal with the ugliness to use the resulting functions. It's not worth the complexity in most cases but if it's something general you're going to use a lot (and lots of C libraries have this idiom of Resource_Init Resource_Destroy function pairs) then it can be worthwhile. Eigen is a quintessential example. It's some of the most complex and inscrutable template code out there but it makes writing numerical code in C++ almost as nice as matlab while also being extremely fast.
- masklinn 13y agoWell the ugliness here comes from the creation of a completely generic wrapper generator for any (constructor, destructor) C pair. If you just want to wrap one pair, you can typedef on unique_ptr and you're done
- devcodex 13y agoHaha I wouldn't call myself a smart dev, just one that's trying to learn how to write good library code. If you really want to make your eyes bleed look at the proposed implementation of just such a utility: http://isocpp.org/files/papers/N3949.pdf http://isocpp.org/files/papers/N3949.pdf Behind that wonderful STL code lies some crazy code behind the scenes. Is it beautiful? You won't catch me taking that argument! But is it typesafe with minimal overhead and easy for the end user of the code to understand and use? That's the one that's most important. For all the complexity of the template function itself, using it comes down to this: auto window = sdl2::make_window("App Name", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, 640, 480, 0); Not an angle bracket in sight!
- mattgreenrocks 13y agoWrapper classes + RAII is superior to this approach: 1. Easier to read 2. Easier to use 3. Doesn't rely on bleeding-edge compiler features I used to do some fun stuff with templates back in the day (CRTP), but I had trouble reading this. Usually, the number of discrete resources you have to wrap this way is fairly small. I tend to cheat and write header-only libraries because the classes are tiny, don't change much, and shouldn't be used in every compilation unit if you have any semblance of a decent architecture. Edit: clarified that I mean wrapper classes due to nerd sniping
- masklinn 13y ago> RAII is superior to this approach: unique_ptr is not RAII now? > 1. Easier to read That's debatable, here ``make_resource`` has to be understood once and will thereafter work with any C constructor/destructor pair. Considering the tone of your comment I guess you mean wrapping each pair in its own object so each of these wrapper objects (and its ownership semantics) has to be understood, and at a fundamental level they're redundant with unique_ptr. If you don't like the generic thingy and want one wrapper object per call pair, you can just typedef the unique_ptr. > 2. Easier to use Easier to use than calling a function and having a unique_ptr with clear ownership semantics?
- mattgreenrocks 13y agoDon't appreciate your tone. Relax buddy, it's Friday. I'm arguing for introducing thin abstractions over the underlying API to handle lifetime issues, and encapsulate icky, SDL-specific details and types as they arise. They are definitely redundant with unique_ptr. In return, they're higher-level and more abstract. It's up to the developer to select what level they need to be at. I favor abstraction and readability, others may not.
- masklinn 13y ago> Don't appreciate your tone. Relax buddy, it's Friday. Wow, sorry, I didn't know you were an internet badass, that changes everything. As for relaxing, have you considered taking your own advice? > I'm arguing for introducing thin abstractions over the underlying API to handle lifetime issues, and encapsulate icky, SDL-specific details and types as they arise. Yes, now that you point it out I can certainly see a careful balancing of the tradeoffs between having to maintain custom code versus the ability to create more extended abstractions in your summary declaration that unique_ptr is hard to read, hard to use and not RAII.
- onedognight 13y agoThis article starts with a flawed premise, that the right way to create a managed pointer is something like this[1]. std::unique_ptr<void,void(*)(void*)> big_ptr(malloc(1), free); The wasteful thing about this is that you have to store the deleter (free() in this case) right next to your void* for every pointer you manage, so the unique_ptr is twice as large as it needs to be. What you'd like to do is store the deleter in the type of the pointer rather than the pointer itself. It turns out unique_ptr can handle this. If the second template parameter is a type with an overloaded function call operator, then unique_ptr will use that to delete. We can wrap it up like so. template <class T, void F(T *)> struct unique { struct deleter { void operator()(T *p) { F(p); }; }; typedef std::unique_ptr<T, deleter> ptr; }; With this helper type you can now declare a similar managed pointer unique<void, free>::ptr small_ptr(malloc(1)); but this time notice that the deleter function is the second template parameter and not an argument to the constructor and the following holds. assert(2 * sizeof(small_ptr) == sizeof(big_ptr)); [1] I used malloc() and free() so those of you following along at home can easily try this out for yourselves.
- deleted 13y ago[deleted]
- onedognight 13y agoHere's a less reusable version that I hope is more understandable. First create an otherwise empty type with an overloaded function call operator that just calls free() on its argument struct freer { void operator()(void *p) { free(p); }; }; and can be used like so. freer f; f(malloc(1)); // This will malloc() and then call free() With this type you can now declare your pointer like this. std::unique_ptr<void, freer> still_small_ptr(malloc(1)); Of course both kinds of unique_ptrs, fat and skinny, can useful. Simulated virtual objects for example might need custom destructors per pointer. However most uses I've seen want the skinny one while they implement the fat one because it's simpler to declare.
- azth 13y agoWhoops, deleted my previous comment since I quickly found out the answer after I submitted the question :) The compiler already knows the location of the function, and should be able to call it directly, if not inline it. Thanks for your answer.
- nly 13y agounique_ptr is not really a good solution to this problem. Something like Boost.ScopeExit for short lived resources, or shared_ptr (which stores the deleter using type erasure, meaning you don't need to allow evidence of the deleter to to permeate through your codebase) is a lot easier on the eyes... template <typename... Args> inline std::shared_ptr<SDL_Window> CreateWindow (Args&&... args) { return std::shared_ptr<SDL_Window> {SDL_CreateWindow(std::forward<Args>(args...)), &SDL_DestroyWindow}; } auto window = CreateWindow ("App Name", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, 640, 480, 0); You know, or someone should write a good quality wrapper like you would just expect for any other language (Python, Ruby, whatever). Many here seem to be blasting C++ when nobody is doing any better without a similar level amount of work elsewhere.
- lholden 13y ago'shared_ptr' can use type erasure because it is a much heavier object. 'unique_ptr' on the other hand is supposed to be a nearly free. If you wanted a version of your code with unique_ptr, you would create a SDLDelete struct template. The result would be nearly identical except for an additional type parameter (and the new struct). Then if you wanted to make "CreateWindow" generic for any of SDL's create/delete pairs... you are basically looking at the OP's make_resource code but slightly different. :P Their version just avoids making a new struct.
- tdicola 13y agoThis is a clever use of unique_ptr, but I can't help but think this is missing the forest through the trees. There's a lot of difficult to read code so you can use a C library resource with a bunch of C APIs. If you wrap the window in a class you can get simple RAII semantics, and add a friendly more C++ API. What if you want to add event handlers for window events, change the window title, or even (gasp) throw exceptions instead of constantly check return codes? Wrapping the resource in a class gives you a great place to add this functionality. Without the class you have a nice resource that won't leak but still feels like you're programming C.
- michaelrmmiller 13y agoIf you were creating a C++ wrapper, you still might prefer to use this same technique. Make the unique_ptr-wrapped resource a member of the wrapper instead of the raw resource. That would give the wrapper the same semantics as unique_ptr without any extra work. If you wanted shared ownership, use shared_ptr instead. Lastly, you wouldn't need to manually declare a destructor to free the resource. In the end, I find it more readable to see unique_ptr and shared_ptr than raw resources. They describe more than just resource management.
- okamiueru 13y agoMy own way of doing this: https://github.com/swarminglogic/sdl2-cross/blob/master/src/util/RAIIhelper.h https://github.com/swarminglogic/sdl2-cross/blob/master/src/... https://github.com/swarminglogic/sdl2-cross/blob/master/src/util/SDL.h https://github.com/swarminglogic/sdl2-cross/blob/master/src/... Certainly less verbose, and solves the problem quite nicely as far as I've been using it. If someone with some wisdom feel like commenting on it, I'm not averse to learning.