Detecting if 2 Iterators point to the same thing?

Started by
39 comments, last by Sean_Seanston 9 years ago

This could be done like this:
&(*itA) == &(*itB)
Dereferencing the iterators to get (references to) the actual elements, and then getting the addresses of those actual elements in memory, and comparing those addresses.


If those two iterators are from different containers, how could that ever evaluate to true? Unless you can have containers somehow overlapping in memory...


*facepalm* I wasn't thinking clearly.

It is *possible* using custom allocators, but using an allocator to allocate the *same* memory location for two *different* containers would be really weird.

I wish I could find the place where I read it again (although it could have been in a C not C++ context or relational operators), but I remember comparing pointers to separately allocted objects being pointed out as not well-defined in the standard.
Lining your path with traps for which you need to be a language lawyer is one of the things making C/C++ annoying.

Yes, I was pointing how how it could be done, and immediately followed up with one potential pitfall - multiple inheritance giving different addresses to ""the same"" instance (the only pitfall that came to my mind), and a better solution - using unique IDs for each object, if the OP is storing classes he can alter the definitions of. I don't advocate using memory addresses as unique identifiers for general purposes (though sometimes it is useful) - I should've been more explicit in that regard. smile.png

Advertisement

I remembered later too that even resizing a container will invalidate an iterator

Sometimes. Not always. Each function for each container has different guarantees about whether it'll invalidate the iterators, and which iterators it'll invalidate.

A basic example is std::vector::pop_back(). That'll invalidate the last iterator (and the end() iterator). But it also guarantees it won't invalidate the other iterators.

See std::vector::pop_back(), and the section labeled "Iterator validity". Each function's page has the details for that function/container. You don't have to worry about memorizing that detail, only remembering to *check*, and overtime you'll remember the details for the ones you use most frequently.

so clearly iterators are very much linked to the state of their particular containers.

Yes. Iterators and pointers to the raw memory, depending on the container.

The point of iterators and pointers pointing at elements is to hold onto them for a short time for the use of a single function or algorithm. For referring to objects long-term, you need a different method - that's not iterators' purpose (and dangerous for pointers to), because containers need to shuffle around their objects in memory.

Guess one way of getting around it is just by testing if we're dealing with the same container first, then continuing as appropriate.

I think you took the wrong off-ramp, and need to get back on the highway for another couple exits before making your turn.

What I mean is, take a step back and don't ask "how can I check if the iterators refer to the same object", and instead ask, "What is my actual goal again? How can I go about implementing that?"

- Have an inventory that stores how many of whatever items someone/something has.
- Item type is referred to using a string.
- Items have a condition value referring to their state of repair, from 1-5.
- The quantity of items stored is recorded using an int.



Personally, I wouldn't use a string to identify them, but I'll do it that way below anyway.

You have two conflicting 'requests' for this container. The quantity of items is stored (great! So all items of the same type are identical!), but differing durability/condition on each item (oh, so they're not identical?). Since they aren't actually identical (they have different conditions), each item (even of the same type) should be different elements. You don't need compression or need to micro-manage the memory here, items are very small, memory-wise.

An easy straightforward way to doing this is:


struct Item
{
     std::string name;
     Enum type;
     int condition = ...;
     Stats stats;
};
 
typedef std::vector<Item> Items;
typedef size_t ItemIndex;
typedef ItemIndex InvalidItemIndex = ItemIndex(-1);
 
//Convenience functions. Iterates over the container and looks up an index using the item name.
//If multiple items exist with the same name, returns the first one found. If no item in 'items' exists with that name, returns 'InvalidItemIndex'.
ItemIndex FindFirstIndexWithName(const Items &items, const std::string &name);
 
//If it exists, "erases" that index from 'items'. To prevent other indices from becoming invalid,
//that item's name is just set to "" (an empty string), so we know that spot is available for re-use.
void EraseItemByIndex(Items &items, ItemIndex indexToDelete);
 
//Adds an item to the container and returns its new index.
ItemIndex AddItem(Items &items, Item itemToAdd)
{
      //Check for the first available spot we can re-use, by looking for an item with an empty string name (which means we previously erased it).
      ItemIndex availableIndex = FindFirstIndexWithName(items, "");
 
      if(availableIndex != InvalidItemIndex)
      {
           items[availableIndex] = itemToAdd;
           return availableIndex;
      }
      
      //If the index returned is invalid, that means we need to create a new spot.
      //This potentially invalidates any *iterators*, but our indices are still fine.
      items.push_back(itemToAdd);
 
      //The new index is the final index in the container.
      return (items.size() - 1);
}
 
struct Player
{
     ItemIndex equippedHelm = InvalidItemIndex;
     ItemIndex equippedChestPiece = InvalidItemIndex;
     ItemIndex equippedWeapon = InvalidItemIndex;
    
     Items inventory;
};


I'd actually do it a bit more complicated than this, probably using std::unordered_maps, unique IDs, and more layers of redirection, but for a simple game, there's no point in overcomplicating it.

The key is, make it simple, and write convenience functions to do the heavy lifting for you.


When adding an item, just search through the inventory vector to look for entries that have a matching name/condition, and then combine the amounts instead of adding a new item. You'd still have to write that code anyway with your map (i.e. check if an item already exists, and then merge instead of add), so it doesn't really get you anything.

Hmm... yes, that's true. Hadn't really got to that part yet unfortunately.

I'll probably go with that then. Using maps can be very verbose too, especially if you're not using C++11 like I'm not, and especially when that map contains a pair...


If you stated what rendering API you're using, I'm sure people would have suggestions.

Oh yeah, I'm using OpenGL.


And there's no reason to loop through a map looking for something you already have an iterator to. Looking up stuff is what maps are for. map.find(key) should work fine to see if that particular key is in there.

Well what I was doing was iterating through the whole map to print out the name/condition/quantity of each item to the screen. The iterator was for detecting which item should be "selected" both in visual terms on screen and in terms of being the item to manipulate in the case of moving etc.


The point of iterators and pointers pointing at elements is to hold onto them for a short time for the use of a single function or algorithm. For referring to objects long-term, you need a different method - that's not iterators' purpose (and dangerous for pointers to), because containers need to shuffle around their objects in memory.

I'll be sure to keep this in mind in future. Makes sense.


The quantity of items is stored (great! So all items of the same type are identical!), but differing durability/condition on each item (oh, so they're not identical?). Since they aren't actually identical (they have different conditions), each item (even of the same type) should be different elements. You don't need compression or need to micro-manage the memory here, items are very small, memory-wise.

I think I might have been averse to making another class unless absolutely necessary at the time I planned out the system... I remember working on a project a few years ago which got bloated to hell with classes and a needlessly complicated architecture that may have irrationally scared me away from another class in this case...

Probably for the best to have another class. Especially since it's conceivable that condition won't be the only distinguishing attribute for a stored item in the end.

I'll think about all this and see what I come up with. Thanks all.


Personally, I wouldn't use a string to identify them, but I'll do it that way below anyway.

Regarding this, why is that exactly BTW?

Is it just that you might envision a situation where more than 1 item type might have the same name, or is it something else?

What I've actually been planning has been what is probably best described as the Flyweight pattern, which I've also used before.

What I was going to do exactly was mark the Inventory entry with the std::string to signify the type, and then I would have looked up a table of ItemType objects which described all of the characteristics common to that type.

As it is now, since I'm replacing the map with a vector of Item objects, I'm going to just give the Item class a shared_ptr< ItemType > member that refers to the ItemType object found in a map of all ItemTypes keyed by an std::string.

So I'll then just have some kind of getType() function in the Item class, and use that to access its name etc. when necessary.

Is that closer to what you were thinking about, or was it something completely different?

Compared to an enum, a std::string:

- takes up more space

- costs more to hash

- allocates heap memory

- is "weakly-typed"... if you mis-type the string name for an item somewhere in your code, you won't find out until runtime, instead of compile time

Why not just use an enum? It addresses all those problems.

Now, if you need to be able to dynamically add new types of inventory items without recompiling your code (like, by just editing a game data file), then you will need some kind of late-binding like a string would offer. Or, you could dynamically generate a new integer id or something. Depends on how your game is setup.

Maybe rename ItemType to ItemCharacteristics, or ItemDescription.

Personally, I wouldn't use a string to identify them, but I'll do it that way below anyway.


Regarding this, why is that exactly BTW?


Because I'd want to load data from files, I'd want each item to have a unique ID, and unique integers are easier to generate than unique strings (just increment ), and unique gibberish strings serve no benefit. And yes, I'd occasionally want more than one item to share the same display name - I can think of at two unique situations where I'd want that, if the gameplay desired that feature.

Ideally*, the code wouldn't be hard-coding any item names or IDs anyway - but scripts or other data files might.

*Ideally. wink.png

What I've actually been planning has been what is probably best described as the Flyweight pattern, which I've also used before.


Flyweight is excellent when trying to conserve memory, like for textures and song files and such. But each item is only a tiny piece of memory - even if you had 20,000 items, it still wouldn't be a problem. You'd be trading code complexity for memory conservation in an area where you don't need memory conservation.

What I was going to do exactly was mark the Inventory entry with the std::string to signify the type, and then I would have looked up a table of ItemType objects which described all of the characteristics common to that type.

Certainly, and that's still doable. Using the string as a lookup to a template item "type" is a reasonable thing to do. I just wouldn't use a string for each item instance, since you'll be creating additional item instances at runtime.

As it is now, since I'm replacing the map with a vector of Item objects, I'm going to just give the Item class a shared_ptr< ItemType > member that refers to the ItemType object found in a map of all ItemTypes keyed by an std::string.


That's also a reasonable solution. Although, at the risk of complicating things, I'd suggest a simple raw pointer instead of a smart pointer. Don't get me wrong, I like smart pointers! But if you are loading all your item types at startup, then you've probably guaranteed that all your item types will outlive your item instances. In which case, a raw pointer or reference is also perfectly acceptable.

Is that closer to what you were thinking about, or was it something completely different?

Something like that. My actual implementation would depend on my gameplay needs. There are a number of ways to do it, which are all fine.

- allocates heap memory


That's not true like this. First, where any memory needed comes from can be specified with an adequate allocator. Second, many standard library implementations have short string optimizations where short (what that means depends on the implementation) strings do not need any additional memory allocated.
If you want to compare two different types of object instance by value, if it were me I'd deference the iterators to get those values and implement operator == for comparing the object types together in a way that makes sense to you, e.g. By id or name...

Now, if you need to be able to dynamically add new types of inventory items without recompiling your code (like, by just editing a game data file), then you will need some kind of late-binding like a string would offer.

Yeah, that was part of the idea behind the decision. That I'd presumably store the various item types in external files eventually, like how I used XML for a project previously. So an enum seemed out of the question.

Ultimately, I expect there to be a lot of different item types, as I'm working towards a system of buying/selling not terribly unlike Elite etc. So that's one reason why being able to bunch together similar items and manipulate them en masse is important. You might be selling 200 of an item, and that item could have a base value of 500, but because they have a condition of 2/5, they might each be worth 200 or w/e. Or some people may not want to buy them at all. That sort of thing. Still working on the exact design as I go of course...


And yes, I'd occasionally want more than one item to share the same display name - I can think of at two unique situations where I'd want that, if the gameplay desired that feature.

Just the way I've been viewing it so far... is that if I ever had 2 types of item that theoretically could have the same display name... it seems like I'd be just as well to give them different names, even if it was something like "AK-47" vs "AK-47 (Gold)" or w/e.

Mind you, one other way I was sorta thinking of was along the lines of

- You might have an item called "AK-47" representing the standard version of that item.

- You might want certain special variants... one made of gold, like I mentioned, or one that shoots faster.

- Perhaps it would be simpler in terms of display wrt fitting names on screen and maybe cleaner in general to just have them all fall under the name of something like:

"AK-47(+)".

- You could then maybe give them unique descriptions if necessary etc.

- However I'm not completely sold on the idea, because while it does do a good job of marking that something is different to the norm, it doesn't actually say how in any way from the name alone...

...but on the other hand, in many cases it would likely take quite a few words at minimum to properly express its unique characteristics and so make that impractical to fit inside a name and would be better in some form of description attribute.

I guess my conclusion is that it would probably be better if I used an incrementing unique numerical ID, as long as it wasn't much hassle.

What would you have in mind? Just read all item data from a file, then have the program automatically generate numerical item type IDs in sequence?

EDIT: So where I'm at right now regarding my inventory architecture is:

- My Inventory class contains a vector, the vector contains objects of the class InvEntry.

- InvEntry represents an inventory entry describing the item contained.

- InvEntry contains an Item object, and a quantity integer representing how many of that Item are stored.

- The Item class describes the item.

Does that all sound good? No way of cleanly getting around the InvEntry class I think while still using a simple vector, and it accounts for Items being differentiated by any number of things in the future by simply updating the == operator overloading and comparing items for equality before shoving them into the same InvEntry.

One more thing...

In order to correctly highlight the selected item, which could be an entry in any random inventory, it seems I'll need a way of marking which inventory is currently the selected one (and then I can use the index to access the correct item in that, otherwise the index isn't much use).

Given that an inventory is defined as:

std::vector< InvEntry > invEntries;

How should I go about storing some sort of reference to which inventory is the one containing the current item?

It might be practical for me to infer it through other means depending on what I know about the game's state... but that could get messy/time-consuming compared to just being able to test whether or not an Inventory is one that I've previously remembered as being selected.

Since I can't think of any other way OTOH... would it be fine to just use a shared_ptr< std::vector< InvEntry > >? Seems like that would make sense.

This topic is closed to new replies.

Advertisement