3 ms·

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

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)

10098 13y ago
Maybe 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 ago
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]. https://news.ycombinator.com/item?id=7610571 https://news.ycombinator.com/item?id=7610571
angersock 13y ago
Yeah, 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 ago
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: 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?
- pbsd 13y ago
  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...))>;
- devcodex 13y ago
  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!
devcodex 13y ago
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?

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)

10098 13y ago

Maybe 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 ago

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]. https://news.ycombinator.com/item?id=7610571 https://news.ycombinator.com/item?id=7610571

angersock 13y ago

Yeah, 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 ago

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: 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?

pbsd 13y ago
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...))>;
devcodex 13y ago
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!

devcodex 13y ago

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?