That for loop crashes the program... Need help!Code:void prodRemove(string name, vector<product> vec){ for(vector<product>::size_type i = 0; i != vec.size(); ++i){ if(vec[i].name == name){ vec.erase(vec.begin() + i); } } }
Thanks in advance
That for loop crashes the program... Need help!Code:void prodRemove(string name, vector<product> vec){ for(vector<product>::size_type i = 0; i != vec.size(); ++i){ if(vec[i].name == name){ vec.erase(vec.begin() + i); } } }
Thanks in advance
Currently research OpenGL
Suppose i = 7 and vec.size = 8 (so we're looking at the last element on the list) and we happen to want to erase it. So we erase it, making vec.size 7; incrementing i gives us 8; i != vec.size, so we try to access vec[8] and bang!
Ahh... So I set i to 0 again? Or can I use break; in For loops?
EDIT: nvm, tried myself :P
Currently research OpenGL
You can break if you know there's only going to be one element you want to delete. (As it is you're skipping the element after the one you delete anyway.)
Setting i to 0 is bad because then you have to start over.
What I had in mind is that you would use a real condition in your for loop, to wit "<" instead of "!=".
Yeah, thought about that too, but the user inputs a name, and I only want the first found product having that name, incase the user has inputted the same product twice, so break suits me fine
Currently research OpenGL
Unless the vector is passed by reference that code will have no effect once the function ends as the vector operated upon will be the local copy and not the original... or is that just a simple omission in the posted code?
"Owners of dogs will have noticed that, if you provide them with food and water and shelter and affection, they will think you are god. Whereas owners of cats are compelled to realize that, if you provide them with food and water and shelter and affection, they draw the conclusion that they are gods."
-Christopher Hitchens
Yeah, I noticed that :P But it's changed now
Currently research OpenGL
This is how the loop should look like:
That is, you only move on to the next element if you didn't remove anything.Code:void prodRemove(string name, vector<product>& vec){ for(vector<product>::size_type i = 0; i != vec.size(); ){ if(vec[i].name == name){ vec.erase(vec.begin() + i); } else { ++i; } } }
However, it is recommended to use existing algorithms instead of coding loops yourself. Sadly, since the condition for removal is not trivial (compare against a member) and if you cannot use lambda functions, you'll need a function object. The following code is a bit longer, but everything there is simpler and easier to get right than in the original code. It is also more efficient performance wise since it erases from the end of the vector and remove_if doesn't move anything more than once.
Code:class NameEquals { std::string name; public: NameEquals(const std::string& n): name(n) {} bool operator()(const product& prod) const { return prod.name == name; } }; void prodRemove(const string& name, vector<product>& vec) { vec.erase( remove_if(vec.begin(), vec.end(), NameEquals(name)), vec.end() ); }
I might be wrong.
Quoted more than 1000 times (I hope).Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
Why is break; not good enough o.O? It's also way clearer, easier to understand, or atleast I think so xP
Currently research OpenGL
Perhaps if there is only one product by the name and you want to remove just that.
Again, it could be done like this:
Code:class NameEquals { std::string name; public: NameEquals(const std::string& n): name(n) {} bool operator()(const product& prod) const { return prod.name == name; } }; void prodRemove(const string& name, vector<product>& vec) { vec.erase(find_if(vec.begin(), vec.end(), NameEquals(name))); }
I might be wrong.
Quoted more than 1000 times (I hope).Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
Well I want it to be like, let's say some store used this and added a product called Soda, then later, maybe after adding loads of other products, adds Soda again, 'cause the user forgot he already had inputted it, so he wants to remove it, then he only has to type /remove, and then Soda, and it only deletes one of the Soda's
break; does that fine enough, and it's blue in VC :P
Currently research OpenGL
If you want one of each kind at any time, you can use a std::set<product>. Then any time you add a Soda, any previous Soda if any will be replaced.
You'll need to overload operator< or provide a comparison function.
I might be wrong.
Quoted more than 1000 times (I hope).Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
"Owners of dogs will have noticed that, if you provide them with food and water and shelter and affection, they will think you are god. Whereas owners of cats are compelled to realize that, if you provide them with food and water and shelter and affection, they draw the conclusion that they are gods."
-Christopher Hitchens