5 ms·

I think relying on the destructor of the moved-from object running feels idiomatic to me. That's what the destructor is there for after all.

olliej 8y ago
The destructor deleting its /own/ data. But here we are moving a bunch of the fields (and so clearing them) but swapping one of them. That seems like bad coding, and doesn’t match any idiom I’ve seen :-/
- jcelerier 8y ago
  for vectors it makes std::move a very fast operation
  - AstralStorm 8y ago
    Vector has a good move assignment which also works. Why swap when you can move? Swap is additionally risky because someone can ADL specialize it for their own purposes. (which is the point) And suddenly your move assignment is broken. So people work around it by adding using std::swap clause there... And then you're calling all those destructors.
  - olliej 8y ago
    std::move on vector zeroes out the source element array. e.g. (pseudo-ish code) operator=(vector&& foo) { delete [] elements; elements = foo.elements; foo.elements = nullptr; } what the example code they have as an answer does is something morally equivalent to this: operator=(vector&& foo) { swap(elements, foo.elements);' } relying on the knowledge that because it's being moved the ~vector is going to be called immediately afterwards. This strikes me as being "clever" in the "potentially confusing code, for no real reason" sense
    - gpderetta 8y ago
      actually that move assignment operator is not ideal. The foo parameter won't be destroyed immediately but only at the end of whatever scope (if any) it was at the call site. The idiomatic copy and swap is like this: T& operator =(T x) // note pass by value { std::swap(this, x); return this; } This acts both as a normal and move assignment operator (if T is not copy constructible); edit: how do you get HN to show asterisks? There should be one before each 'this'
      - arh68 8y ago
        Prefix each line with 4 spaces: T& operator =(T x) // note pass by value { std::swap(*this, x); return *this; }
- archi42 8y ago
  I wasn't too sure on some of the easy questions, but this swap... Oh god, my "someday, this is going to shoot someone in the foot"-sense started to tingle.
- lemmyg 8y ago
  Move assignment has nothing to do with 'clearing fields' or whatever it's that the moved-to value steals what it can and leaves the moved from value in a valid but unspecified state. Notably, you should still be able to assign-to and destroy a moved-from value. It's an invariant of the 'DirectorySearchResult' class that query is non-null, as the destructor deletes it without checking it for nullity. The most straight-forward and efficient way to achieve both the stealing and the valid state is to swap the pointers.
  - jemfinch 8y ago
    > It's an invariant of the 'DirectorySearchResult' class that query is non-null, as the destructor deletes it without checking it for nullity. Deleting a null pointer is a fully specified no-op in C++.
    - lemmyg 8y ago
      That's true. And I see the other example answers they have written do assume the pointer can be null, although the original class makes no indication of this. To make the question clearer they should have added a function on the original class that checks (or doesn't check) the pointer before dereferencing it, thereby clarifying whether query being null is valid or not.

olliej 8y ago

The destructor deleting its /own/ data. But here we are moving a bunch of the fields (and so clearing them) but swapping one of them. That seems like bad coding, and doesn’t match any idiom I’ve seen :-/

jcelerier 8y ago
for vectors it makes std::move a very fast operation
- AstralStorm 8y ago
  Vector has a good move assignment which also works. Why swap when you can move? Swap is additionally risky because someone can ADL specialize it for their own purposes. (which is the point) And suddenly your move assignment is broken. So people work around it by adding using std::swap clause there... And then you're calling all those destructors.
- olliej 8y ago
  std::move on vector zeroes out the source element array. e.g. (pseudo-ish code) operator=(vector&& foo) { delete [] elements; elements = foo.elements; foo.elements = nullptr; } what the example code they have as an answer does is something morally equivalent to this: operator=(vector&& foo) { swap(elements, foo.elements);' } relying on the knowledge that because it's being moved the ~vector is going to be called immediately afterwards. This strikes me as being "clever" in the "potentially confusing code, for no real reason" sense
  - gpderetta 8y ago
    actually that move assignment operator is not ideal. The foo parameter won't be destroyed immediately but only at the end of whatever scope (if any) it was at the call site. The idiomatic copy and swap is like this: T& operator =(T x) // note pass by value { std::swap(this, x); return this; } This acts both as a normal and move assignment operator (if T is not copy constructible); edit: how do you get HN to show asterisks? There should be one before each 'this'
    - arh68 8y ago
      Prefix each line with 4 spaces: T& operator =(T x) // note pass by value { std::swap(*this, x); return *this; }
archi42 8y ago
I wasn't too sure on some of the easy questions, but this swap... Oh god, my "someday, this is going to shoot someone in the foot"-sense started to tingle.
lemmyg 8y ago
Move assignment has nothing to do with 'clearing fields' or whatever it's that the moved-to value steals what it can and leaves the moved from value in a valid but unspecified state. Notably, you should still be able to assign-to and destroy a moved-from value. It's an invariant of the 'DirectorySearchResult' class that query is non-null, as the destructor deletes it without checking it for nullity. The most straight-forward and efficient way to achieve both the stealing and the valid state is to swap the pointers.
- jemfinch 8y ago
  > It's an invariant of the 'DirectorySearchResult' class that query is non-null, as the destructor deletes it without checking it for nullity. Deleting a null pointer is a fully specified no-op in C++.
  - lemmyg 8y ago
    That's true. And I see the other example answers they have written do assume the pointer can be null, although the original class makes no indication of this. To make the question clearer they should have added a function on the original class that checks (or doesn't check) the pointer before dereferencing it, thereby clarifying whether query being null is valid or not.