Wouldn'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.
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?
The 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?
The 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.
My 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)
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.
I 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].
This 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:
The standard dictates that decltype(*r) deduces to T&, not T. The std::decay is there to collapse the reference type into a value type. Therefore your simplification results in a compilation error.
An alternative way to get the value type is to use std::remove_pointer_t:
using ResourceType = std::remove_pointer_t<decltype(c(args...))>;
Thank you for taking the time to write this out, you hit the head exactly on what I was trying to accomplish with this article. I saw an opportunity to make a reusable tool for myself while at the same time being able to work on some of the best practices I've seen for developing library code. The real payoff is in the usage code.
Your simplification of the original code is great! I like how pulling out the creation of the the resource to its own line makes the typedef and unique_ptr initialization much easier to read (as far as templates go that is, a real sticking point here today).
I have updated my article and attributed the clarification to you, thanks again for your feedback and comments!
The function that does all the work is 5 lines and then it works for any resource created from Init/Destroy function pair (a very common practice in C libraries), that's a hideous amount of code to you?
With your experience, how would you write a general solution for this problem?
I 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.
This 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.
Well 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
Haha 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:
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);
Wrapper 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
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?
Don'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.
> 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.
The comment you responded to was indeed inappropriately irritating. But please don't respond to incivility with more incivility on HN. It's hard not to, but it only makes things worse.
I agree with you, I would personally stick with a wrapper class unless there was some ungodly number of resources to wrap. There are very likely other operations on a window object which will benefit from being able to wrap it in a class with more points for extension.
std::unique_ptr is a class that wraps around a pointer for the express purpose of providing RAII semantics. How is it not a wrapper class? All you're proposing is writing a class that does what unique_ptr does only now without templates each type has to be written out by hand.
Do you not ever make use of any of the STL templates like std::vector because their implementations are hard to read? Probably not, because they are easy to use and you never have to worry about how they are implemented. Well, here is how you use my function, please tell me how I could make this function call any easier because I honestly would like to know.
auto window = sdl2::create_window("New Window", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, SCREEN_WIDTH, SCREEN_HEIGHT, 0);
I wrote a short function that I can reuse with any C based Init/Destroy created resource. Now that it works I never have to think about any of the template magic that went into writing it because using it is dead simple... there aren't even any angle brackets in the call. Not a single proposed "better" solution in this thread has shown the capability of creating resources in the general manner, each one shows the creation of a concrete class which means it has to be duplicated for each type. Which defeats the point of the article.
If you worked with templates back in the day then you probably aren't too familiar with the parameter pack syntax or auto return type deduction, these are new things and yes they are bleeding edge. How is anyone suppose to understand and learn good practices for using these features if they don't make use of them and write about it for others to provide feedback on?
I wrote this article to learn about specific techniques and get feedback on them. It's C++, there's a million ways to skin a cat here... and this article was specifically to address C++14 features. That's why it wasn't titled "C++03 and SDL". I thank you for taking the time to comment but telling someone they shouldn't make use of new compiler features because you aren't familiar with them and then not providing any sort of example yourself to the proposed problem doesn't come across as constructive at all.
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.
Here'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
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.
Whoops, 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.
The other downside to their approach is that you have to pass the function into the constructor every time you construct an object.
A similar alternative would be to template the deleter on the type of the function and then a specific function. This way you can wrap functions with non-void return values. Something like:
The other cool thing I learned recently is you can actually use the custom deleter to override the wrapped type by typedef'ing pointer in your deleter class. You can use this to wrap C-style file descriptors in a unique_ptr.
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:
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!)
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.
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.
> 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
In the examples given, the function pointers in question would be stored on the stack. How many windows should an application be juggling at once that an extra 8 bytes per window is a concern? This seems like a premature optimization to me.
If you're individually rendering blades of grass in a goat simulator, sure. But by the time you're dealing with arbitrary-length application-defined strings (like window names), an extra pointer isn't a big deal.
For me to believe that a small_ptr was worthwhile here, I would have to see some benchmarks, which is how good optimization happens anyway.
Thanks for the comment and suggestion. How might this look in the general solution though? The type of the resource has to be deduced somewhere and I wasn't able to figure out a good way to deduce that and declare this style of deleter within the std namespace.
I'm all for eliminating that extra bit of overhead and don't claim to be an expert, the whole purpose for me writing this article was to specifically learn about how to write reusable library code. If you have any suggestions for how to pull this off in the general solution I'm all ears!
unique_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...
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.
'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.
This 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.
If 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.
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.
45 comments
[ 1.3 ms ] story [ 89.7 ms ] threadAlso, 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.
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?
(At least the errors have got slightly better in recent g++ releases with colourised error output.)
You don't have to, you can create further helper function calling the generic thing but it's optional.
In these proposed solutions I've not yet seen one that has the capability to do this:
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?
I suggest that this is somewhat solving the wrong problem.
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.
Another use on here suggested a shorter and more efficient way to write the code [0].
https://news.ycombinator.com/item?id=7610571
You end up exposing the SDL functionality as a normal class object, and everything's much more straightforward.
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:
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:
And again, written sloppily: Or, if you don't mind repetition of ResourceType: Clearer?An alternative way to get the value type is to use std::remove_pointer_t:
Your simplification of the original code is great! I like how pulling out the creation of the the resource to its own line makes the typedef and unique_ptr initialization much easier to read (as far as templates go that is, a real sticking point here today).
I have updated my article and attributed the clarification to you, thanks again for your feedback and comments!
With your experience, how would you write a general solution for this problem?
Want to look at some tasteful C++ code? Check out LLVM.
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.
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:
Not an angle bracket in sight!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
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?
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.
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.
Do you not ever make use of any of the STL templates like std::vector because their implementations are hard to read? Probably not, because they are easy to use and you never have to worry about how they are implemented. Well, here is how you use my function, please tell me how I could make this function call any easier because I honestly would like to know.
auto window = sdl2::create_window("New Window", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, SCREEN_WIDTH, SCREEN_HEIGHT, 0);
I wrote a short function that I can reuse with any C based Init/Destroy created resource. Now that it works I never have to think about any of the template magic that went into writing it because using it is dead simple... there aren't even any angle brackets in the call. Not a single proposed "better" solution in this thread has shown the capability of creating resources in the general manner, each one shows the creation of a concrete class which means it has to be duplicated for each type. Which defeats the point of the article.
If you worked with templates back in the day then you probably aren't too familiar with the parameter pack syntax or auto return type deduction, these are new things and yes they are bleeding edge. How is anyone suppose to understand and learn good practices for using these features if they don't make use of them and write about it for others to provide feedback on?
I wrote this article to learn about specific techniques and get feedback on them. It's C++, there's a million ways to skin a cat here... and this article was specifically to address C++14 features. That's why it wasn't titled "C++03 and SDL". I thank you for taking the time to comment but telling someone they shouldn't make use of new compiler features because you aren't familiar with them and then not providing any sort of example yourself to the proposed problem doesn't come across as constructive at all.
Thanks for your answer.
A similar alternative would be to template the deleter on the type of the function and then a specific function. This way you can wrap functions with non-void return values. Something like:
The other cool thing I learned recently is you can actually use the custom deleter to override the wrapped type by typedef'ing pointer in your deleter class. You can use this to wrap C-style file descriptors in a unique_ptr.That's why the suggest a make_window function at the bottom of the post. The end-user just calls:
The optimizer should inline all the extra wrappers away, so there's no overhead for doing as the article suggests.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:
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: 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!)
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.
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.
In the examples given, the function pointers in question would be stored on the stack. How many windows should an application be juggling at once that an extra 8 bytes per window is a concern? This seems like a premature optimization to me.
If you're individually rendering blades of grass in a goat simulator, sure. But by the time you're dealing with arbitrary-length application-defined strings (like window names), an extra pointer isn't a big deal.
For me to believe that a small_ptr was worthwhile here, I would have to see some benchmarks, which is how good optimization happens anyway.
I'm all for eliminating that extra bit of overhead and don't claim to be an expert, the whole purpose for me writing this article was to specifically learn about how to write reusable library code. If you have any suggestions for how to pull this off in the general solution I'm all ears!
Thanks again for taking time to comment!
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.
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.
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.
https://github.com/swarminglogic/sdl2-cross/blob/master/src/...
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.