-1

This is the first time I use this idiom and I have a vector v1 which elements must be a subset of the elements in another vector v2. Now, imagining v1 and v2 as sets, I want to perform v2-v1 and throw an exception if v1[i] doesn't exists in v2 (for any meaningful i).

I came up with this:

std::vector<T> v1;
std::vector<T> v2;
//fill v1, v2 somehow
for(size_t i=0; i<v1.size(); i++){
  v2.erase(std::remove(v2.begin(), v2.end(), v1[i]), v2.end());
  if(/*erase option failed because v1[i] doesn't exists in v2*/)
    throw std::runtime_exception (std::to_string(i)+"-th in v1 doesn't exists in v2!");
}

How can I fill the if condition?

justHelloWorld
  • 6,478
  • 8
  • 58
  • 138
  • use std::includes, if the vectors are of can be sorted. –  May 18 '17 at 08:49
  • @manni66 I specified that they are not sorted because they can't be (the order must be preserved) – justHelloWorld May 18 '17 at 08:52
  • If there are a million items, it would more than likely be more efficient to pass both vectors by value to a function, std::sort those temporary vectors, and call something like `std::mismatch`. Your code would loop 10^12 iterations on unsorted data, while sorting two temporary copies of a million items takes far less time. – PaulMcKenzie May 18 '17 at 09:11
  • @PaulMcKenzie Thanks for your answer. This is not the case, we are talking about thousands of elements in `v1`. For this particular part of the code (where efficiency is not the priority) I prefer to keep a simple and clear code. I think that just because we're using C++ doesn't mean that we have to use the most efficient solution *always*, don't you agree? ;) – justHelloWorld May 18 '17 at 09:13
  • What is not clear about `bool isDifferent(vector v1, vector v2);` and inside that function, `sort` and call `std::equal` or `std::mismatch`? Looking at your code, it needs to be looked at 2 or three times to figure out what it is doing (it isn't self documenting). Scott Parent discusses this in many of his excellent videos, where writing hand-coded `for` loops is less advantageous than using the various algorithm functions to do the job. – PaulMcKenzie May 18 '17 at 09:16

1 Answers1

8

Simply check if any elements were removed:

const auto orig_size = v2.size();
v2.erase(std::remove(v2.begin(), v2.end(), v1[i]), v2.end());
if (v2.size() == orig_size)

Or split it up (nothing forces you to do the remove and erase in a single statement):

const auto last = std::remove(v2.begin(), v2.end(), v1[i])
if (last == v2.end())
  throw std::runtime_exception (std::to_string(i)+"-th in v1 doesn't exists in v2!");
v2.erase(last, v2.end());

You could make the code far more efficient if you keep both vectors sorted and use an algorithm to walk through them together. Your current code is O(n²) because it searches the whole of v2 for every element in v1.

Jonathan Wakely
  • 166,810
  • 27
  • 341
  • 521