map stl class wrapper

Started by
7 comments, last by DrEvil 18 years, 2 months ago


class Lua :public Singleton<Lua>
{
public:
	typedef std::multimap<std::string, std::string> MapStr;
	typedef std::multimap<std::string, std::string>::iterator MapStrIterator;

public:
	Lua();
	~Lua();

	std::vector<std::string>* itemSameType(std::string type);

 ..............

//data
	lua_State * m_lua;
	MapStr m_data;
};

std::vector<std::string>* Lua::itemSameType(std::string type)
{
	MapStr::iterator iter = m_data.lower_bound(type.c_str());
	
	std::vector<std::string> *tmp = new std::vector<std::string>;

	if (iter != m_data.end())
	{
		for (int i=0;i<count(type);i++)
		{
			tmp->push_back( iter->second );
		}

		return tmp;
	}
	else
	{
		return 0;
	}
}


take a look itemSameType() I think it is not good design. who give me better idea? thanks. 1.new easy to forgot to release. [EDIT]2.if I return a std::vector<std::string> variable(not pointer), is it effective in performance? ........
Advertisement
Quote:
2.if I return a std::vector<std::string> variable, is it effect?


i realize than english is not your native language, but if that sentence means "will it work", then yes.
Can you clarify what the purpose of this class is?

If it is something to do with lua I recommend storing the map in lua tables. It will probably be more efficient memory-wise because I believe lua does string references, so if there are multiple occurances of a particular string, many different places can reference the same string. Storing it in a C++ map will end up storing multiple copies of the string. Plus you would have flexibility of holding any data type within the table.
I won't comment on he actual usage of te class and instead provide a commented code-snipped that improves things a little:
class Lua :public Singleton<Lua>{        // NOTE: public constructors defeat Singleton-semantics!        Lua();        // NOTE: avoid copy construction as well, by declaring, but not defining         //       the constructor/assignment operator        Lua( Lua const & );        Lua & operator = ( Lua const & );public:	typedef std::multimap<std::string, std::string> MapStr;	typedef std::multimap<std::string, std::string>::iterator MapStrIterator;             public:	~Lua();        // returning by value makes much more sense here, also watch out        // for const-correctness	std::vector<std::string> itemSameType(std::string const & type) const; ..............//data	lua_State * m_lua;	MapStr m_data;};std::vector<std::string> Lua::itemSameType(std::string const & type) const{        // using .c_str() would result in an implicit conversion to a temporary string object - avoid that	MapStr::const_iterator iter = m_data.lower_bound(type);	std::vector<std::string> result;        // I assume you actually wanted to copy all mapped string into the         // resulting output vector, not just the first one:        for ( ; iter != m_data.end() && iter->first == type; ++iter  )     	     result.push_back( iter->second );            // please note that multimap:lower_bound will return an iterator to                     // the first mapped value who's key is equal OR greater than the provided        // parameter. therefore you'll have to test not only for .end() but        // also for a matching key value.        return result;}


HTH,
Pat.
Seems like you might have an answer now, but this isn't really DirectX related [smile]

For reference purposes or just for any continuation, I think this belongs in General Programming instead.

Cheers,
Jack

<hr align="left" width="25%" />
Jack Hoxley <small>[</small><small> Forum FAQ | Revised FAQ | MVP Profile | Developer Journal ]</small>

Quote:Original post by DrEvil
Can you clarify what the purpose of this class is?

If it is something to do with lua I recommend storing the map in lua tables. It will probably be more efficient memory-wise because I believe lua does string references, so if there are multiple occurances of a particular string, many different places can reference the same string. Storing it in a C++ map will end up storing multiple copies of the string. Plus you would have flexibility of holding any data type within the table.


I do store data in table of lua. then I copy table to stl map because map has many function to do many thing. operation on table is so complex.
So it just fills in a vector with entries that match a given string.

I would have the user pass the vector by reference, and allow the function to fill it in. Otherwise the function creates a vector + strings as temporaries, returning by value copies them all. I would change it to.

void Lua::itemSameType(std::string const & type, std::vector<std::string> &_list) const

This would avoid temporaries and making a copy and be faster. Plus would open up the opportunity for someone to use the same vector and pass to multiple calls to the function to accumulate more results(if that'd be useful). There are compiler optimizations(return value optimization I think it's called) that could avoid the temporaries but IMO it's better to not rely on them.

You might consider multimap.equal_range instead of doing the additional check yourself with using lower_bound
Quote:Original post by darookie
I won't comment on he actual usage of te class and instead provide a commented code-snipped that improves things a little:
*** Source Snippet Removed ***

HTH,
Pat.


Do you mean when return result ,do not return all data in vector?

so is return vector similar to return vector* in performance?
Returning a new'd vector pointer has the problem that someone has to delete it at some point.

Returning the vector(by value) is slow because it has additional copying steps.

IMO it's best for the function to pass in the vector to be filled in, because then the caller can make the vector on the stack and have the function fill it in, or the caller can pass it a vector that it keeps persistantly. Plus it remains clear that the lifetime of the vector is the responsibility of the caller.

This topic is closed to new replies.

Advertisement