Jump to content
  • Advertisement
Sign in to follow this  
revanthedarth

std::set issue

This topic is 3323 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I'm trying to use std::set in my resource manager because inserting and searching is logn. I'm trying to store pointers because I will have interfaces for resources like ITexture (ITexture has std::string name, std::set should compare using name). Then if engine uses OpenGL, CGLTexture will be loaded. So if I use std::set<ITexture>, whole thing would be wrong because of the inheritance stuff. If I use std::set<ITexture*>, it will sort the list by adress and it would be meaningless. If I use std::set<ITexture*, ITexture> and code a function for ITexture ( bool operator<(const ITexture*, const ITexture*) ), std::set creates ITexture once and destroys twice for no reason. Enginuity influenced me a lot when I started to build the engine, so I use some of its classes.
class ITexture : public IMMObject
{
public:
	std::string name;
	int id;
	ITexture(){ITextureNum++; id = ITextureNum; printf("%d WAS CREATED!\n", id);}
	virtual ~ITexture(){printf("%d WAS DESTROYED!\n", id);}
	bool operator()(const ITexture*, const ITexture*);
	static int ITextureNum;
};
class CResourceManager : public Singleton<CResourceManager>
{
public:
	CResourceManager(){printf("RM created\n");}
	~CResourceManager(){}
private:
	std::set<ITexture*, ITexture>textures;
};

bool ITexture::operator()(const ITexture* lhs, const ITexture* rhs)
{
	printf("HEY HEY HEY!\n");
	printf("(%s < %s)?\n", lhs->name.c_str(), rhs->name.c_str());
	return (lhs->name < rhs->name);
}


When the CResourceManager is created, I get these outputs: 1 WAS CREATED 1 WAS DESTROYED 1 WAS DESTROYED RM created It does not print "HEY HEY HEY!" but it creates a ITexture and destroys it twice or something like that, i really don't get why and how it happens. When I use std::set<ITexture*>, it doesn't create or destroy ITexture.

Share this post


Link to post
Share on other sites
Advertisement
You should consider taking the name out of the texture and using a map<std::string, ITexture> to access you textures by name. The resulting code is probably easier.

Share this post


Link to post
Share on other sites
Quote:
Original post by revanthedarth
IIf I use std::set<ITexture*>, it will sort the list by adress and it would be meaningless. If I use std::set<ITexture*, ITexture> and code a function for ITexture ( bool operator<(const ITexture*, const ITexture*) ), std::set creates ITexture once and destroys twice for no reason.
Make a comparator functor (which can be an inner class of ITexture, if you like) to do the sorting.

Share this post


Link to post
Share on other sites
If you want to explore this more, you'll want to overload operator< instead of
operator(), and use a free friend function if you plan to specify both lhs and rhs.

All the same, as alvaro says, this is a problem more appropriate to std::map if you plan to request textures by string name. std::map isn't a hash_map, so it does keep things in key order, if that's a requirement.

Share this post


Link to post
Share on other sites
You are not taking into account the copy constructor.


#include <set>
#include <cstdio>

int counter = 0;

class Predicate
{
int id;
public:
Predicate(): id(++counter)
{
printf("create %d\n", id);
}
Predicate(const Predicate& rhv): id(++counter)
{
printf("copy %d: %d\n", rhv.id, id);
}
~Predicate()
{
printf("destroy %d\n", id);
}
bool operator()(int a, int b) const
{
return a < b;
}
};

int main()
{
std::set<int, Predicate> s;
}




For me, this alone creates an instance of Predicate, and a copy of it.

Predicates should be rather light-weight objects. set<ITexture*, ITexture> is bizarre - you need a whole ITexture instance - or more - to be able to compare two ITexture*'s?!

(The map idea is probably better.)

Share this post


Link to post
Share on other sites
Quote:
Original post by revanthedarth
If I use std::set<ITexture*>, it will sort the list by adress and it would be meaningless.


I have an application that does just this, and it works fine. Providing the address of your textures never changes, ie. they're not elements in a vector or something which is going to reallocate them, then the memory address is a valid way to sort the elements.

Quote:
You should consider taking the name out of the texture and using a map<std::string, ITexture> to access you textures by name. The resulting code is probably easier.

Be careful with this; I once profiled the same application, which used to use this method, and in some cases it was CPU-bound due to all the calls to map::find. Use names if you like, but whenever possible, especially during heavy rendering, only refer to them by their address.

Share this post


Link to post
Share on other sites
Your mistake was to make the comparison function the operator() of the ITexture class. The std::set creates an instance of ITexture to use as the comparer [you might find that the default constructor of std::set<T,Fn> takes a single parameter of type Fn and default is to Fn()].

* It never calls the comparison function because you aren't storing any ITextures in the std::set
* It calls the default constructor once, the destructor twice, and the copy constructor once


You should probably be posting this in for beginners

Share this post


Link to post
Share on other sites
AshleysBrain, thanks, but I was planning to use ResourceManager's GetTexture function which loads the texture if it doesn't exist. So if I was going to use addresses for comparing, I would end up loading a texture more than once.

I didn't think about the copy constructor, thanks people. By the way, which way would you use, a big class that has lots of comparing functions or seperate classes for each comparable object?

Share this post


Link to post
Share on other sites
For every class that has a "natural" comparison method, provide an operator< overload. For each additional "special" comparison method, write a separate class (or struct) with an operator() overload.

Keep in mind that you don't ever have to write, e.g. an operator> overload unless your class has very strange comparison semantics. Just write operator< and/or operator==; the rest can be (and are) synthesized in terms of those two. E.g.


// This is actually provided for you by one of the standard library headers, IIRC
template <typename X, typename Y>
bool operator!=(const X& x, const Y& y) {
return !(x == y); // invoke X::operator==(const Y&) const, and negate the result
}

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!