The most efficient way to erase from a set when iterating over it

Question

What is the most efficient way to erase from a set, iterate over it? Here are two approaches that I thought was better between. Is there any better way?

void WaitForFiles(std::set<string> files) {
  while (files.size() > 0) {
    std::set<string> found_files;
    for (const auto& file : files) {
      if (Exists(file)) {
        found_files.insert(file);
      }
    }
    for (const auto& found_file : found_files) {
      files.erase(file);
    }
  }
}

Using set_difference:

void WaitForFiles(std::set<string> files) {
  while (files.size() > 0) {
    std::set<string> found_files;
    for (const auto& file : files) {
      if (Exists(file)) {
        found_files.insert(file);
      }
    }
    std::set<string> difference;
    std::set_difference(files.begin(), files.end(),
                        found_files.begin(), found_files.end(),
                        std::inserter(difference, difference.end()));
    files = difference;
  }
}

Please note that the following failures:

void WaitForFiles(std::set<string> files) {
  while (files.size() > 0) {
    for (const auto& file : files) {  // <-- seg fault after first erase
      if (Exists(file)) {
        files.erase(file);
      }
    }
  }
}

To determine the effectiveness, keep in mind that in my case the file may take 30 minutes, so the Exists function will be called many times, and the installed files will not change very often compared to the number of times the cycle repeats.

Answer 1

undefined ( ). , , .

std::set::erase std::set, :

for(auto itr = files.cbegin(); itr != files.cend();) {
  if (exists(*itr)) {
    std::cout << "Found file: " << *itr << "\n";
    itr = files.erase(itr);
  } else
    ++itr;
}

-.

Answer 2

std::experimental::erase_if v2 TS:

std::experimental::erase_if(files, Exists);

Exists , lambda:

std::experimental::erase_if(files, [](const auto& f) { return Exists(f); });

VS2015.

Answer 3

, . , , , std::set::erase.

I would suggest that the following should be quite effective. However, as others have said, it is worth replacing std::set<std::string> found_fileswith std::vector.

#include <set>
#include <string>
#include <iostream>

/**
 * Return true for every other call
 */
bool Exists(std::string const&)
{
    static int i = 0;
    return ++i % 2;
}

std::set<std::string> find_existing_files(std::set<std::string>& files)
{
    std::set<std::string> found_files;

    for(auto file = files.begin(); file != files.end();)
    {
        if(!Exists(*file))
            ++file; // not erasing, keep going
        else
        {
            found_files.insert(*file);
            // replacing iterator with result of erase()
            // keeps the iterator valid
            file = files.erase(file);
        }
    }
    return found_files;
}

int main()
{
    std::set<std::string> files {"a", "b", "c", "d", "e"};

    for(auto const& file: find_existing_files(files))
        std::cout << "exists : " << file << '\n';

    for(auto const& file: files)
        std::cout << "missing: " << file << '\n';
}

Output:

exists : a
exists : c
exists : e
missing: b
missing: d