-1

I'm trying to remove objects from a vector while looping over it. I tried to implement "Erase-remove idiom" but still can't get it to work, I've made several re-writes to try make it work but still end up getting error.

Here's the function meant to remove objects from the vector (parameter 1)

void ta_bort_person(std::vector <Person>& telReg, std::string fnamn){
int j = 0;
int index;
bool continueLooping = true;
Person tempPerson;
while (continueLooping) {
    for (int i = 0; i < telReg.size(); i++) {
        if (telReg[i].fnamn == fnamn) {
            std::string enamn = telReg[i].enamn;
            std::string nummer = telReg[i].nummer;
            std::cout << fnamn << " " << enamn << " tas nu bort." << std::endl;
            tempPerson = telReg[i];
            break;
            //tempList.push_back(person);
        }
        continueLooping = false;
    }
    if (continueLooping) {
        telReg.erase(std::remove(telReg.begin(), telReg.end(), tempPerson), telReg.end());//this is where I get error
    }
}

} Person struct:

struct Person
{
std::string fnamn;
std::string enamn;
std::string nummer;
};

Message from compiler:

C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.27.29110\include\xmemory(2018,28): error C2676: binary '==': 'Person' does not define this operator or a conversion to a type acceptable to the predefined operator
1>C:\Users\chris\source\repos\Labb1-CPROG\Labb1-CPROG\Source.cpp(99): message : see reference to function template instantiation '_FwdIt std::remove<std::_Vector_iterator<std::_Vector_val<std::_Simple_types<_Ty>>>,Person>(_FwdIt,const _FwdIt,const _Ty &)' being compiled
1>        with
1>        [
1>            _FwdIt=std::_Vector_iterator<std::_Vector_val<std::_Simple_types<Person>>>,
1>            _Ty=Person
1>        ]
1>Done building project "Labb1-CPROG.vcxproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========
273K
  • 29,503
  • 10
  • 41
  • 64
Loathed
  • 35
  • 1
  • 4
  • 3
    Define `operator ==` for `Person` structure. It is required by [`std::remove`](https://en.cppreference.com/w/cpp/algorithm/remove) for comparison. Flagging this as off-topic: _"While similar questions may be on-topic here, this one was resolved in a way less likely to help future readers."_ – brc-dd Nov 09 '20 at 14:51
  • https://en.cppreference.com/w/cpp/algorithm/remove *"1) Removes all elements that are equal to value, using operator== to compare them. "* – JohnFilleau Nov 09 '20 at 14:52
  • 2
    The compiler told you exactly what to do. Related: [https://stackoverflow.com/questions/10070020/c-operator-overloading](https://stackoverflow.com/questions/10070020/c-operator-overloading) also [https://stackoverflow.com/a/4421719/487892](https://stackoverflow.com/a/4421719/487892) – drescherjm Nov 09 '20 at 14:56
  • I hadn't learned about operator-methods yet, thanks. – Loathed Nov 09 '20 at 16:06
  • 1
    My hope is to get you to begin to understand what the compiler is telling you. With that said I do know compiler error messages can be intimidating at times especially for those new to the language. – drescherjm Nov 09 '20 at 16:29

1 Answers1

1

This seems clumsy. I wrote the below to test something much cleaner. In short, I defined an operator== that compares against a string.

#include <iostream>
#include <vector>
#include <string>

class Person {
public:
    std::string fnamn;
    std::string enamn;
    std::string nummer;
};

bool operator==(const Person &person, const std::string &name) {
    return person.fnamn == name;
}

void dump(std::vector<Person> &persons) {
    for (Person &person: persons) {
        std::cout << "Person: " << person.fnamn << std::endl;
    }
}

int main(int, char **) {
    std::vector<Person> persons;

    persons.push_back(Person{"Joe", "Cool", "10"});     // Appears 3 times
    persons.push_back(Person{"Fred", "Smith", "20"});
    persons.push_back(Person{"Joe", "Cool", "11"});
    persons.push_back(Person{"Sam", "Spade", "30"});
    persons.push_back(Person{"Joe", "Cool", "12"});

    std::cout << "Beginning:" << std::endl;
    dump(persons);
    persons.erase(std::remove(persons.begin(), persons.end(), "Joe"), persons.end());

    std::cout << "At end:" << std::endl;
    dump(persons);
}

Output is:

$ g++ --std=c++14 TestRemove.cpp -o TestPerson
$ TestPerson
Beginning:
Person: Joe
Person: Fred
Person: Joe
Person: Sam
Person: Joe
At end:
Person: Fred
Person: Sam

If I modify to also print the size, I get 5 at the start and 2 at the end.

But then I modified it:

#include <iostream>
#include <vector>
#include <string>

class Person {
public:
    std::string fnamn;
    std::string enamn;
    std::string nummer;
};

void dump(std::vector<Person> &persons) {
    for (Person &person: persons) {
        std::cout << "Person: " << person.fnamn << std::endl;
    }
}

int main(int, char **) {
    std::vector<Person> persons;

    persons.push_back(Person{"Joe", "Cool", "10"});     // Appears 3 times
    persons.push_back(Person{"Fred", "Smith", "20"});
    persons.push_back(Person{"Joe", "Cool", "11"});
    persons.push_back(Person{"Sam", "Spade", "30"});
    persons.push_back(Person{"Joe", "Cool", "12"});

    std::cout << "Beginning: " << persons.size() << std::endl;
    dump(persons);
    persons.erase(std::remove_if(persons.begin(), persons.end(), [](const Person &person) { return person.fnamn == "Joe";}), persons.end());

    std::cout << "At end: " << persons.size() << std::endl;
    dump(persons);
}

In this version, I got rid of the operator== and used remove_if with a lambda, which I think is probably a better implementation.

$ g++ --std=c++14 TestRemove.cpp -o TestPerson && TestPerson
Beginning: 5
Person: Joe
Person: Fred
Person: Joe
Person: Sam
Person: Joe
At end: 2
Person: Fred
Person: Sam
Joseph Larson
  • 8,530
  • 1
  • 19
  • 36
  • Either of these get rid of your double-nested loop and are significantly cleaner, a 1-line implementation of your method, possibly with an operator== defined. – Joseph Larson Nov 09 '20 at 15:10
  • Thank you, with your answer to guide me I solved my problem. – Loathed Nov 09 '20 at 16:06
  • I am glad you have got your solution and thanks for your sharing, I would appreciate it if you mark them as answer and this will be beneficial to other community. – Barrnet Chou Nov 18 '20 at 09:01
  • Yeah, @Loathed, the next time you're on, accept Joseph's answer please. – Aaron Bell Jan 16 '21 at 06:59