searching vector
Hello, what is a fast way to search a vector to see if it contains a user defined class(it has an operator== only for other instances of the same class). I only need to know if it has one instance of the class.
ok, heres the code for what I used:
and here are the errors:
bool character::hasitem(const item& it){ vector<item> copy=this->getinventory()->getitems();//gets a copy of the vector of items for this instance vector<item>::iterator it=find(copy.begin(),copy.end(),it); if(it==copy.end()) { return false; } else { return true; }}
and here are the errors:
//about 5 of these errors207 C:\Dev-Cpp\include\c++\3.4.2\bits\stl_algo.h no match for 'operator==' in '(&__first)->__gnu_cxx::__normal_iterator<_Iterator, _Container>::operator* [with _Iterator = item*, _Container = std::vector<item, std::allocator<item> >]() == __val'
First, do me a favor and name your iterator something other than the function parameter and see if the compiler still complains.
Is it a new error, or the same error?
You could use count(vec.begin(), vec.end(), some_item) == 1 if you don't actually need the item afterwards.
You could use count(vec.begin(), vec.end(), some_item) == 1 if you don't actually need the item afterwards.
First, style issues:
- Assuming a good reason to separate 'inventory' from 'character', the finding process really belongs to the inventory. The character can then "delegate" to its inventory, rather than grabbing the raw item list (which really belongs to the inventory and should be better encapsulated than that).
- Don't use an if/else to return false or true; just return the appropriate boolean expression directly. Concise is clear.
- Don't copy data when you don't need to; beyond any performance concerns, it's just plain messy, and falsely "documents" the function as possibly changing something and then either committing or rolling-back. find() is a non-mutating algorithm.
Second, please show your operator== implementation; I suspect the problem lies there.
- Assuming a good reason to separate 'inventory' from 'character', the finding process really belongs to the inventory. The character can then "delegate" to its inventory, rather than grabbing the raw item list (which really belongs to the inventory and should be better encapsulated than that).
- Don't use an if/else to return false or true; just return the appropriate boolean expression directly. Concise is clear.
- Don't copy data when you don't need to; beyond any performance concerns, it's just plain messy, and falsely "documents" the function as possibly changing something and then either committing or rolling-back. find() is a non-mutating algorithm.
// I assume "items" member of inventory that is returned normally by getitems()// and similarly "inv" member of character, of type inventorybool inventory::contains(const item& it) { return find(items.begin(), items.end(), it) != items.end();}bool character::hasitem(const item& it) { return inv.contains(it);}
Second, please show your operator== implementation; I suspect the problem lies there.
Quote:Original post by jdhardy
Is it a new error, or the same error?
You could use count(vec.begin(), vec.end(), some_item) == 1 if you don't actually need the item afterwards.
That's actually less efficient than using find() since find() will return as soon as it reaches a match while count() check each element in the vector. Also, that assumes there is only one match in the vector. The condition should probably be > 0 not == 1.
ok, I have changed my implementation of hasitem to zahlman's idea.
here is my operator== code:
EDIT: fixed; it was the const correctness in the operator=='s. I am bad with those things [grin]
here is my operator== code:
bool operator==(item& i,item& it){ if(i.getname()==it.getname()) { return true; } return false;}bool operator==(item *i,item& it)//dont ask why i have a pointer version; it is part of the game.{ if(i->getname()==it.getname()) { return true; } return false;}
EDIT: fixed; it was the const correctness in the operator=='s. I am bad with those things [grin]
The function signatures should probably be different. That is:
bool operator==(item& i,item& it)
should probably be:
bool operator==(const item& i, const item& it)
bool operator==(item& i,item& it)
should probably be:
bool operator==(const item& i, const item& it)
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement