3

I am writing a class which requires highly efficient function for filtering member container (lets say std::vector). This function should have interface similar to the following:

void filter(std::vector<SomeType>& items, const std::vector<int>& inds);

The function should leave the container items in the following state: - items that are pointed to by the index from inds should be removed - other items should remain in container keeping initial order.

For simplicity lets assume that inds is a perfect container with O(1) on every operation and all indices are valid with no duplication.

My idea was to create the second container, reserve required space and then move (through std::move) every element not indexed by inds into this new container; and afterwards just swap the old and the new containers.

For example like this:

void filter(std::vector<SomeType>& items, const std::vector<int>& inds)
{
    std::vector<SomeType> new_items{};
    new_items.reserve( items.size() - inds.size() );

    for(size_t i = 0; i < items.size(); ++i)
        if( contains( inds, i ) ) // magic O(1) function, never mind
            new_items.push_back( std::move( items[i] ) );

    items.swap(new_items);
}

My questions are:

1) after using std::move on some element inside the vector (or generally any other standard container) will there be any issues like double destructing of these elements?

2) Is there a standard way to do such filtering efficiently?

dodekja
  • 537
  • 11
  • 24
Glib
  • 304
  • 1
  • 13
  • 1
    you don't need two vectors, as you are just removing, so you can move first item after removed after the last previously remaining item and from there keep copying all items which are not to be removed, and skipping over items which should be removed. This may have slightly better performance if your vector is very large (halving the memory usage), or if you are removing only few items, i.e. the initial skip to first item to be removed (not modifying the initial part) may save a tiny fraction of time. I.e. 12345678 (minus 3 5 7) = 12468 -> you can write "468" over original "345678" safely. – Ped7g Aug 27 '19 at 12:41
  • BTW the `std::vector` by nature requires type which can be "moved" ("T must meet the requirements of CopyAssignable and CopyConstructible."), as the `vector` itself will reallocate the whole buffer when it runs out of space, and "moves" all values from one memory buffer to the new one (usually memmove-like in case of simple types, copy-assignment in case of more complex types). I.e. if your `SomeType` is already correct for usage inside `std::vector`, the `std::move` should work for it too I think (hopefully I didn't forget about some formal detail why `std::move` may fail on such type). – Ped7g Aug 27 '19 at 12:51

2 Answers2

8

It's not the container's responsibility to safeguard from issues that might arise from moving. As long as the type of the items moved has a well-defined and correct move constructor / implicit move constructor then moving the items from items to new_items is the same as any other move operation; containers don't change this.

In short, this responsibility lays with the class, not the container it's used in.

Hatted Rooster
  • 35,759
  • 6
  • 62
  • 122
-1

The objects you moved from are placed in a valid but unspecified state, and functions without a precondition can be called on them. And the destructor has to be a function without a precondition. If SomeType does not fulfill this condition then it is ill-formed.

So no there would not be a problem with a double destruction.

t.niese
  • 39,256
  • 9
  • 74
  • 101
  • 2
    *The objects you moved from are placed in a valid but unspecified state, and functions without a precondition can be called on them* This is only true if that is what the class's move constructor provides. If they provide a string guarantee, then you have a strong one. If you forget about the rule of 5 and don't have a move constructor in a resource allocating class then you have no guarantee. The container will "do the right thing", only if the object does. – NathanOliver Aug 27 '19 at 12:21
  • 2
    Re: "The objects you moved from are placed in a valid but unspecified state, and functions without a precondition can be called on them" -- that's a requirement that the C++ standard imposes on types defined in the standard library. It's a good design policy, but it is only true for user-defined types if they in fact are implemented to do that. – Pete Becker Aug 27 '19 at 12:50