I'm trying to delete an item from a vector with erase() function but I keep getting an error. I
searched everywhere but can't find an answer
#include <iostream>
#include <vector>
#include <map>
#include <iterator>
#include <algorithm>
using namespace std;
class Person{
private:
string name;
public:
void set_name(string name){
this->name = name;
}
string get_name(){
return name;
}
class Record{
private:
vector <Person> book;
public:
void delete_person(string name){
for(Person p : book){
if(book.get_name() == name){
book.erase(p);
}
}
}
};
int main(){
// nothing in main yet
return 0;
}
I get en error in the delete_person() function in the record class: No matching member function for call to 'erase'
CodePudding user response:
book.erase(p);
book
is a vector. The parameter to a vector's erase()
method is an iterator.
for(Person p : book){
p
is the value in the vector, and actually it is a copy of the value in the vector. You cannot pass a value to erase()
. You must pass an iterator as a parameter. Passing some random copy of some random value in a vector to its erase()
method is not going to accomplish anything useful.
std::vector
has begin()
and end()
methods that return the iterator to the beginning and the end of a sequence that defines the contents of the vector.
This may be used with various algorithms, like std::find_if
or std::remove_if
, together with std::vector::erase
to effect the removal of a value or multiple values from your vector.
CodePudding user response:
void delete_person(string name){
for(Person p : book){
if(book.get_name() == name){
book.erase(p);
}
}
}
fails for several reasons.
std::vector::erase
does not accept items, it accepts iterators, locations of items to be removed.
Range-based for loops are very simple and limited in their abilities. They go from start to finish and are extremely intolerant of changes to the container while iterating. If you add or remove an item while iterating it, the hidden bookkeeping used by the loop becomes invalid and the loop breaks. And not the nice break
sort of breaking. They tend to take the whole program down with them.
In Person p : book
p
is a new object that is a copy of an item in book
. It's not the original or a reference to the original in the container. C defaults to values instead of references in almost every case. Unless you specifically request otherwise, you pass by value, return by value, and iterate by value.
Instead, employ the Erase-Remove Idiom. Here is an example with added commentary where I saw it fitting or educational.
#include <iostream>
#include <vector>
#include <map>
#include <iterator>
#include <algorithm>
using namespace std;
class Person
{
private:
string name;
public:
Person(const std::string & name) // added for testing
: name(name) // this is a member initializer list In C all class members
// and base classes must be initialized before the program can
// enter the body of the constructor. This trick allows us to
// initialize members rather than initializing them to their
// defaults (if the type has a default) and then setting them
// inside the body and wasting time doing two things where one
// thing was required
{
}
void set_name(string name) // side note consider saving construction of a new
// string and accepting name by const reference rather
// than by value and potentially making a copy.
// void set_name(const string & name)
// const because we do not intend to change `name`
// and because the compiler can take advantage of the
// promise not to change it in many interesting ways.
{
this->name = name;
}
string get_name() const // const because getters generally should not change the
// object this allows us to keep the class "const-correct"
// side note consider saving construction of a new
// string and returning by const reference rather than
// by value and making a copy.
// const string & get_name() const
{
return name;
}
};
class Record
{
private:
vector<Person> book;
public:
void add_person(const std::string & name) // added for testing
{
book.emplace_back(name);
}
void delete_person(string name) // again consider passing name by const reference
{
book.erase(std::remove_if(book.begin(), // from start of list
book.end(), // to the end
[name](const Person &p)
{
return p.get_name() == name;
}), // moves all items to be removed to the end of the
// list, then returns start of range to erase
book.end()); // erase to the end of the list
// Why erase separately? Because remove functions don't actually remove. They
// move the unwanted values to the end of the list. Looks silly, but much easier
// and safer to write. For example, this won't change the size of the list and
// break loops that count on the size to remain the same.
}
friend std::ostream & operator<<(std::ostream & out,
const Record & rec) // added for testing
{
for (const auto & item: rec.book) // print all items in book
// const because printing should not change
// the printed
// auto to let the compiler figure out the type
// & because we don't want to make a copy
{
out << item.get_name() << '\n';
}
return out;
}
};
int main()
{
Record r;
r.add_person("Bill");
r.add_person("Ted");
r.add_person("Rufus");
std::cout << r << std::endl;
r.delete_person("Ted");
std::cout << r << std::endl; // Ted should now be gone from the list
return 0;
}
Expected output:
Bill
Ted
Rufus
Bill
Rufus