6 ms·

I'd think of smart pointers as function parameters as a code smell.[0] Heap allocations should be managed with an RAII class (of which smart pointers may be an

I'd think of smart pointers as function parameters as a code smell.[0] Heap allocations should be managed with an RAII class (of which smart pointers may be an internal implementation detail), and "sink functions" should take an rvalue reference (which semantically means "take ownership of this parameter"). [0] Whether a resource is allocated on the heap or elsewhere, or, whether a heap-allocated resource has owned or reference-counted semantics, are implementation details that should be abstracted away from your top-level APIs, so you have the flexibility to change them later.

vvanders 9y ago
Honestly, everyone reaches for shared_ptr way too quickly/easily. In the majority of cases it's a sign that you don't have a clear owner of a component and you've just deferred the decision to whatever (non-)deterministic behavior your code has. I've found using unqiue_ptr + proper move semantics tend to lead to cleaner, single-owner architectures that are much more sane to maintain/scale.
- smitherfield 9y ago
  Agreed; probably upwards of 90% of the uses of shared_ptr I've seen (and 70% or so with unique_ptr) are by people who clearly don't understand the C/C++ memory model and are trying to write Java in C++, and half the remainder are by people who've lost track of ownership and lifetimes and paper over it by randomly sprinkling smart pointers throughout the codebase until it sort of works. I'm not against smart pointers where they make sense, it's just whenever I see them I start bracing myself for the horrors that seem to inevitably follow.
  - vvanders 9y ago
    Yeah, it's also one of my big gripes about Java or any managed language. You never know if some jerk library grabbed a reference to your object when you passed it to a function and decides to leak it via holding a ref. It's also why I really like that Rust decided to go RAII/single-ownership and add a ton of friction(via RefCell) to make sure that you really want shared ownership. It forces you to get your house in order up-front which I've found almost always leads to a better design overall.
    - smitherfield 9y ago
      RAII really is a brilliant design, it both (mostly) solves the problem of manual memory management and is easier (for my brain at least) to reason about than deterministic (shared_ptr, Obj-C/Swift) or nondeterministic (managed langs) garbage collection schemes. Once I grokked it I've never looked at managed langs the same way again. In C++'s defense it's not like the language or the compilers encourage you to use shared_ptr, but it's also true (mainly for legacy C-interop cruft) it doesn't do enough to nudge you to write code in a proper RAII style, nor to enforce correctness WRT ownership. I like that Rust forces you to explicitly opt out of RAII and/or single-owner semantics. (Those are the "defaults" in C++ too, but too easy to implicitly or accidentally opt out of). On the other hand you can write Java in any language, and from the little Rust I've read, newcomers from managed langs will often daisy-chain enough smart pointers in a row to put any 13-year-old who just downloaded Unreal Engine to shame. The last time I took a serious look at Rust I thought it took things too far in the other direction; you weren't able to use two (nonoverlapping!) data members of the same struct in a function, and the suggested workaround was to wrap it in a smart pointer. Has that issue been fixed now?
      - vvanders 9y ago
        > The last time I took a serious look at Rust I thought it took things too far in the other direction; you weren't able to use two (nonoverlapping!) data members of the same struct in a function, and the suggested workaround was to wrap it in a smart pointer. Has that issue been fixed now? I think that's largely been addressed. Are you talking about doing something like this? https://play.rust-lang.org/?gist=cf5835d28946df6c5093d57b7ca0aba1&version=stable https://play.rust-lang.org/?gist=cf5835d28946df6c5093d57b7ca...
        smitherfield 9y ago
        It's long enough ago I can't remember exactly what I was trying to do except that it was trivially correct, but yeah I think that was it. Anyway, I'm glad that's been solved; looks like I've got some new weekend plans! :)
- Koshkin 9y ago
  > everyone reaches for shared_ptr way too quickly I dont't think it's as bad as it sounds, and I have an explanation: C++ has evolved to the point when the ease of coding in it is comparable with that of Java; so, more people use C++ as if it was Java, and shared_ptr is the closest thing to garbage collection that C++ standard library provides.
  - vvanders 9y ago
    Oh no, it really is that bad. You haven't lived until you've root-caused a ref cycle 3 libraries deep in a 200kloc+ codebase. This problem also isn't exclusive to C++, I've had nasty libraries in Java add things to the root set silently to the same effect.
Joky 9y ago
> Heap allocations should be managed with an RAII class Isn't this what a `unique_ptr` is (a RAII class that manages Heap allocation)? > sink functions" should take an rvalue reference (which semantically means "take ownership of this parameter"). Why not take by value?
- smitherfield 9y ago
  >Isn't this what a `unique_ptr` is (a RAII class that manages Heap allocation)? See my reply to kllrnohj above: https://news.ycombinator.com/item?id=15872509 https://news.ycombinator.com/item?id=15872509 >Why not [write sink functions that] take by value? That could work in the case where the parameter is a class which is movable but not copyable, but I'd still prefer an rvalue reference in that case because it makes your meaning obvious without having to refer to the class definition or documentation.
partycoder 9y ago
> "sink functions" should take an rvalue reference You mean using move semantics?
- smitherfield 9y ago
  Yes, two phrases meaning the same thing. ("Takes an rvalue reference" is slightly less ambiguous because by-value parameters can also in certain circumstances be move-constructed).
kllrnohj 9y ago
I'd argue that classes with a single field are a worse code smell. A using or typedef would be better, at least then your code isn't littered with unneeded field accesses. I agree that these methods should just take rvalue references, though. Doesn't really matter for unique_ptr, but for other things like shared_ptr it adds up quick.
- smitherfield 9y ago
  >I'd argue that classes with a single field are a worse code smell. A using or typedef would be better Sometimes, but not in this case, because - Smart pointers are nullable, which should be opt-in and not opt-out behavior. - Smart pointers have pointer-like interfaces, which do not read like idiomatic modern C++. A wrapper class `GadgetHandle` can define conversion operators for `Gadget&` and `const Gadget&` so their interface reflects their semantic meaning (a `Gadget`) and hides irrelevant or unsafe implementation details (the operators and methods belonging to the smart pointer class). - As a corollary to the above, you can create several classes `UniqueGadgetHandle`, `SharedGadgetHandle`, `CustomGadgetHandle` and so forth, which enable you to very easily and transparently intermix or alter allocation strategies (even on the stack or statically) for your `Gadget`s, usually without needing to alter any public APIs or more than one section of your codebase. - When your heap-allocated data [structures] don't semantically correspond to the containers provided by the STL or your other library dependencies, I find that chances are you have or may add some custom constructor/destructor logic beyond just new and delete.
- Koshkin 9y ago
  You cannot use a typedef if you intend to create a new type (e.g. one that provides a specific set of methods but otherwise identical to a built-in type).
maxxxxx 9y ago
I find smart pointers super valuable for scenarios where you pass memory through a pipeline. A while I ago had to pass measurement data though a pipe of customizable filters and it was really nice to know that the data would be freed once the last filter was done without writing tracking code. But I agree that they tend to get overused.
- remar 9y ago
  I had a similar conundrum when I was trying to learn modern C++ by writing a network stack. On one hand just wrapping my packet objects with a {shared,unique}_ptr meant that I could freely pass these objects through a pipeline and not worry about lifetime or accidentally freeing somewhere I shouldn't have. Although it seems to me that most people seem to be proponents of actually sitting down and working out object ownership of your classes rather than just being lazy and using smart pointers everywhere. I'm still experimenting with this approach, definitely slows me down right now but I imagine that's because I haven't done it enough. Not going to lie, coming from Java/C#, it's really convenient just being able to 'new' an object and not worry much about its lifetime.
  - maxxxxx 9y ago
    I don't think it's lazy to pass around smart pointers. Obviously you need to understand the consequences but in many cases like pipelines it's a pragmatic solution that saves a lot of code.
klipt 9y ago
> "sink functions" should take an rvalue reference (which semantically means "take ownership of this parameter"). Doesn't that only work when the parameter is movable? For older classes that don't yet have move semantics, unique_ptr is an easy wrapper to give them efficient move semantics, is it not?

vvanders 9y ago

Honestly, everyone reaches for shared_ptr way too quickly/easily. In the majority of cases it's a sign that you don't have a clear owner of a component and you've just deferred the decision to whatever (non-)deterministic behavior your code has. I've found using unqiue_ptr + proper move semantics tend to lead to cleaner, single-owner architectures that are much more sane to maintain/scale.

smitherfield 9y ago
Agreed; probably upwards of 90% of the uses of shared_ptr I've seen (and 70% or so with unique_ptr) are by people who clearly don't understand the C/C++ memory model and are trying to write Java in C++, and half the remainder are by people who've lost track of ownership and lifetimes and paper over it by randomly sprinkling smart pointers throughout the codebase until it sort of works. I'm not against smart pointers where they make sense, it's just whenever I see them I start bracing myself for the horrors that seem to inevitably follow.
- vvanders 9y ago
  Yeah, it's also one of my big gripes about Java or any managed language. You never know if some jerk library grabbed a reference to your object when you passed it to a function and decides to leak it via holding a ref. It's also why I really like that Rust decided to go RAII/single-ownership and add a ton of friction(via RefCell) to make sure that you really want shared ownership. It forces you to get your house in order up-front which I've found almost always leads to a better design overall.
  - smitherfield 9y ago
    RAII really is a brilliant design, it both (mostly) solves the problem of manual memory management and is easier (for my brain at least) to reason about than deterministic (shared_ptr, Obj-C/Swift) or nondeterministic (managed langs) garbage collection schemes. Once I grokked it I've never looked at managed langs the same way again. In C++'s defense it's not like the language or the compilers encourage you to use shared_ptr, but it's also true (mainly for legacy C-interop cruft) it doesn't do enough to nudge you to write code in a proper RAII style, nor to enforce correctness WRT ownership. I like that Rust forces you to explicitly opt out of RAII and/or single-owner semantics. (Those are the "defaults" in C++ too, but too easy to implicitly or accidentally opt out of). On the other hand you can write Java in any language, and from the little Rust I've read, newcomers from managed langs will often daisy-chain enough smart pointers in a row to put any 13-year-old who just downloaded Unreal Engine to shame. The last time I took a serious look at Rust I thought it took things too far in the other direction; you weren't able to use two (nonoverlapping!) data members of the same struct in a function, and the suggested workaround was to wrap it in a smart pointer. Has that issue been fixed now?
    - vvanders 9y ago
      > The last time I took a serious look at Rust I thought it took things too far in the other direction; you weren't able to use two (nonoverlapping!) data members of the same struct in a function, and the suggested workaround was to wrap it in a smart pointer. Has that issue been fixed now? I think that's largely been addressed. Are you talking about doing something like this? https://play.rust-lang.org/?gist=cf5835d28946df6c5093d57b7ca0aba1&version=stable https://play.rust-lang.org/?gist=cf5835d28946df6c5093d57b7ca...
      - smitherfield 9y ago
        It's long enough ago I can't remember exactly what I was trying to do except that it was trivially correct, but yeah I think that was it. Anyway, I'm glad that's been solved; looks like I've got some new weekend plans! :)
Koshkin 9y ago
> everyone reaches for shared_ptr way too quickly I dont't think it's as bad as it sounds, and I have an explanation: C++ has evolved to the point when the ease of coding in it is comparable with that of Java; so, more people use C++ as if it was Java, and shared_ptr is the closest thing to garbage collection that C++ standard library provides.
- vvanders 9y ago
  Oh no, it really is that bad. You haven't lived until you've root-caused a ref cycle 3 libraries deep in a 200kloc+ codebase. This problem also isn't exclusive to C++, I've had nasty libraries in Java add things to the root set silently to the same effect.

Joky 9y ago

> Heap allocations should be managed with an RAII class Isn't this what a `unique_ptr` is (a RAII class that manages Heap allocation)? > sink functions" should take an rvalue reference (which semantically means "take ownership of this parameter"). Why not take by value?

smitherfield 9y ago
>Isn't this what a `unique_ptr` is (a RAII class that manages Heap allocation)? See my reply to kllrnohj above: https://news.ycombinator.com/item?id=15872509 https://news.ycombinator.com/item?id=15872509 >Why not [write sink functions that] take by value? That could work in the case where the parameter is a class which is movable but not copyable, but I'd still prefer an rvalue reference in that case because it makes your meaning obvious without having to refer to the class definition or documentation.

partycoder 9y ago

> "sink functions" should take an rvalue reference You mean using move semantics?

smitherfield 9y ago
Yes, two phrases meaning the same thing. ("Takes an rvalue reference" is slightly less ambiguous because by-value parameters can also in certain circumstances be move-constructed).

kllrnohj 9y ago

I'd argue that classes with a single field are a worse code smell. A using or typedef would be better, at least then your code isn't littered with unneeded field accesses. I agree that these methods should just take rvalue references, though. Doesn't really matter for unique_ptr, but for other things like shared_ptr it adds up quick.

smitherfield 9y ago
>I'd argue that classes with a single field are a worse code smell. A using or typedef would be better Sometimes, but not in this case, because - Smart pointers are nullable, which should be opt-in and not opt-out behavior. - Smart pointers have pointer-like interfaces, which do not read like idiomatic modern C++. A wrapper class `GadgetHandle` can define conversion operators for `Gadget&` and `const Gadget&` so their interface reflects their semantic meaning (a `Gadget`) and hides irrelevant or unsafe implementation details (the operators and methods belonging to the smart pointer class). - As a corollary to the above, you can create several classes `UniqueGadgetHandle`, `SharedGadgetHandle`, `CustomGadgetHandle` and so forth, which enable you to very easily and transparently intermix or alter allocation strategies (even on the stack or statically) for your `Gadget`s, usually without needing to alter any public APIs or more than one section of your codebase. - When your heap-allocated data [structures] don't semantically correspond to the containers provided by the STL or your other library dependencies, I find that chances are you have or may add some custom constructor/destructor logic beyond just new and delete.
Koshkin 9y ago
You cannot use a typedef if you intend to create a new type (e.g. one that provides a specific set of methods but otherwise identical to a built-in type).

maxxxxx 9y ago

I find smart pointers super valuable for scenarios where you pass memory through a pipeline. A while I ago had to pass measurement data though a pipe of customizable filters and it was really nice to know that the data would be freed once the last filter was done without writing tracking code. But I agree that they tend to get overused.

remar 9y ago
I had a similar conundrum when I was trying to learn modern C++ by writing a network stack. On one hand just wrapping my packet objects with a {shared,unique}_ptr meant that I could freely pass these objects through a pipeline and not worry about lifetime or accidentally freeing somewhere I shouldn't have. Although it seems to me that most people seem to be proponents of actually sitting down and working out object ownership of your classes rather than just being lazy and using smart pointers everywhere. I'm still experimenting with this approach, definitely slows me down right now but I imagine that's because I haven't done it enough. Not going to lie, coming from Java/C#, it's really convenient just being able to 'new' an object and not worry much about its lifetime.
- maxxxxx 9y ago
  I don't think it's lazy to pass around smart pointers. Obviously you need to understand the consequences but in many cases like pipelines it's a pragmatic solution that saves a lot of code.

klipt 9y ago

> "sink functions" should take an rvalue reference (which semantically means "take ownership of this parameter"). Doesn't that only work when the parameter is movable? For older classes that don't yet have move semantics, unique_ptr is an easy wrapper to give them efficient move semantics, is it not?