my function takes in 2 vectors, compares the elements in each vector for duplicates. If duplicate is found, I want to erase that element from the second vector.
I get a compiler error when I call the erase method. I believe it's because Vector<T> is not the same type as vector<int>::iterator but I am not sure how to fix this.
Could someone suggest how would I fix this?
template <typename T>
std::vector<T> operator+(const std::vector<T>& v1, const std::vector<T>& v2)
{
typename std::vector<T> newVectorOne = v1;
typename std::vector<T> newVectorTwo = v2;
for (std::vector<int>::const_iterator i = newVectorOne.begin(); i != newVectorOne.end(); i++)
{
for (std::vector<int>::const_iterator j = newVectorTwo.begin(); j != newVectorTwo.end(); j++)
{
if (*i == *j)
{
newVectorTwo.erase(newVectorTwo.begin() + j); //error here
}
}
}
newVectorOne += NewVectorTwo;
return newVectorOne;
}
error log:
1>------ Build started: Project: Assignment2, Configuration: Debug Win32 ------
1>main.cpp
1>c:\users\rm\documents\comp3512_2\assignments\assignment2\assignment2.h(67): error C2678: binary '+': no operator found which takes a left-hand operand of type 'std::_Vector_iterator<std::_Vector_val<std::_Simple_types<_Ty>>>' (or there is no acceptable conversion)
1> with
1> [
1> _Ty=int
1> ]
1>c:\program files (x86)\microsoft visual studio\2017\community\vc\tools\msvc\14.11.25503\include\vector(332): note: could be 'std::_Vector_iterator<std::_Vector_val<std::_Simple_types<_Ty>>> std::_Vector_iterator<std::_Vector_val<std::_Simple_types<_Ty>>>::operator +(int) const'
1> with
1> [
1> _Ty=int
1> ]
1>c:\program files (x86)\microsoft visual studio\2017\community\vc\tools\msvc\14.11.25503\include\vector(361): note: or 'std::_Vector_iterator<std::_Vector_val<std::_Simple_types<_Ty>>> std::operator +<std::_Vector_val<std::_Simple_types<_Ty>>>(int,std::_Vector_iterator<std::_Vector_val<std::_Simple_types<_Ty>>>)'
1> with
1> [
1> _Ty=int
1> ]
1>c:\program files (x86)\microsoft visual studio\2017\community\vc\tools\msvc\14.11.25503\include\vector(243): note: or 'std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<_Ty>>> std::operator +<std::_Vector_val<std::_Simple_types<_Ty>>>(int,std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<_Ty>>>)'
1> with
1> [
1> _Ty=int
1> ]
1>c:\users\rm\documents\comp3512_2\assignments\assignment2\assignment2.h(67): note: while trying to match the argument list '(std::_Vector_iterator<std::_Vector_val<std::_Simple_types<_Ty>>>, std::_Vector_iterator<std::_Vector_val<std::_Simple_types<_Ty>>>)'
1> with
1> [
1> _Ty=int
1> ]
1>c:\users\rm\documents\comp3512_2\assignments\assignment2\main.cpp(36): note: see reference to function template instantiation 'std::vector<int,std::allocator<_Ty>> operator +<int>(const std::vector<_Ty,std::allocator<_Ty>> &,const std::vector<_Ty,std::allocator<_Ty>> &)' being compiled
1> with
1> [
1> _Ty=int
1> ]
1>Done building project "Assignment2.vcxproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========
The problem is that you are adding two iterators together at this line:
newVectorTwo.erase(newVectorTwo.begin() + j); //error here
Such an operation doesn't exist: you only want one iterator which you increment and dereference, or an incrementing integral offset that you turn into an iterator when deletion is needed. Taking the latter solution as a template, you could change the outer loop so that j is size_t and goes from 0 to newVectorTwo.size() (exclusive).
Keep in mind that erase also invalidates iterators at and after the point of erasure, so your inner loop is undefined behavior if it actually erases anything (other than the last element, since in that case the loop exits immediately after) - so the approach of using j as an incrementing iterator generally won't work as you have it. Using an index into the vector solves that: you have no reference to iterators at all except the temporary one created by begin().
The performance of your solution will be very poor for vectors of any significant size. The two nested loops and the erase call make it an O(n^3) solution (more specifically O(m*n^2) if v1 and v2 have sizes m and n respectively).
A simpler solution would be just to create newVectorTwo initially empty, and then add each element with push_back which is not a duplicate, rather than trying to remove duplicates from a fully populated vector. This push_back runs in O(1) amortized time, this will at reduce it to O(n^2). You could also just get rid of newVectorTwo and append elements directly to newVectorOne and return that. You'd need to change your outermost iteration to use an index though, to get around iterator invalidation.
If you want an expected O(n) (actually O(n + m)) solution, you could populate a std::unordered_map with the contents of v1 and use that for your lookup, rather than use the two nested loops.