Skip to main content
gamedev.net
🔒 Locked

Help shorten code.

Started by andyb716 Jan 9, 2005 at 6:10 PM 10 replies 700+ views
Original Post
andyb716
andyb716
Hi, I am using c++ and I know there is an easier way to do thid with STL but I'm new and confused so if anyone can help please share your ideas. 

//This will render only one item
void ActorFactory::Render(long ID)
{
	//Start the iterator at the begining of the array
	std::vector<Actor*>::iterator it = m_pActors.begin();

	//Go through each element until it comes to the end
	while(it != m_pActors.end())
	{
		//Set the iterator
		Actor *pActor = *it;

		//If this actor has the ID passed in
		//then render it
		if(pActor->GetID() == ID)
		{
			pActor->Render();
			//Break the loop
			break;
		}
		else
		{
			//Increment the iterator
			++it;
		}
	}
}
Holy Fuzz
Holy Fuzz
Why do you need to use an ID as opposed to a pointer to the object to be rendered?
_the_phantom_
_the_phantom_
std::map or hashmap would be faster and smaller.
Store the entries with IDs linking Actor* and do a find() on it, it will return either an iterator to pair with the actor in or .end()
nmi
nmi
http://www.sgi.com/tech/stl/table_of_contents.html

#include <algorithms>class Functor {long ID;Functor(long ID) : ID(ID) {}void operator() (Actor* pActor){		//If this actor has the ID passed in		//then render it		if(pActor->GetID() == ID)		{			pActor->Render();			//Break the loop			break;		}};void ActorFactory::Render(long ID){  Functor func(ID);  for_each(m_pActors.begin(), m_pActors.end(), func);}
andyb716
andyb716
Im passing in the ID because Im using vectors. If I were to pass the pointer I might as well just use the render method of the object. But I want to use an index or sequence of some sort to only render those of what ID's i want.
bytecoder
bytecoder
This is the best I can do with the limited amount of time I have. I suggest chunking the find an actor by id code into another function, but like I said before, I don't have enough time to do it myself. Also know that I didn't just strip out all the comments because I don't like comments, but because that most of them were unecessary and made the code harder to read. Most of it is self-documenting, thus the only comment in there is the one that describes what the function does.
typedef std::vector<Actor*>::iterator actorIterator;// find the actor based on its ID and render itvoid ActorFactory::Render(long ID) {    for(actorIterator actorIt = m_pActors.begin();        it != m_pActors.end();        ++it) {                Actor *pActor = *it;	if(pActor->GetID() == ID)	{	    pActor->Render();	    return;	}    }}

EDIT:
Just so you know, I did consider using the for_each approach shown earlier in this thread, but I decided not to as I believe that it adds more complexity than it removes for people not used to that approach.
Basiror
Basiror
well then get used to it

a simple line

//this uses a functor and calls the render functions

is all you need to make it understandable for other programmers
http://www.8ung.at/basiror/theironcross.html
Zipster
Zipster
The best way to do this is using a std::map or stdext::hash_map, which has already been mentioned. No surprise, since these structures are designed for key/value situations just like this, which appears to me what the OP is doing since he's using IDs to identify actors. So, rather than wrapping a fuggly functor around a std::vector linear-time lookup, you use the clearer implicit key/value design of an associative container to perform a logarithmic- or constant-time lookup, depending on whether you use a std::map or stdext::hash_map respectively. Did I mentioned that insertion and removal operations have the same complexities? Much better than an std::vector in all cases. And if he decides to have multiple objects use the same ID for grouping purposes, then std::multimap and stdext::hash_multimap fit the bill.

And of course, these containers provide bidirectional iteration, so if you need to perform something on the whole container, it's not a problem.
Drew_Benton
Drew_Benton
Quote:
Original post by andyb716
Hi, I am using c++ and I know there is an easier way to do thid with STL but I'm new and confused so if anyone can help please share your ideas.

*** Source Snippet Removed ***

//This will render only one itemvoid ActorFactory::Render(long ID){for( std::vector<Actor*>::iterator it = m_pActors.begin(); it != m_pActors.end(); it++ )  {  if( (*it)->GetID() == ID)    {        pActor->Render();        //Break the loop        break;    }  }}

That's how I would shorten that code. If you did not want to use a break you could even do this:

//This will render only one itemvoid ActorFactory::Render(long ID){bool done = 0;for( std::vector<Actor*>::iterator it = m_pActors.begin(); it != m_pActors.end() && done == 0; it++ )  {    if( (*it)->GetID() == ID)    {        pActor->Render();        //Break the loop        done = 1;    }  }}
Holy Fuzz
Holy Fuzz
I will agree that a hashtable (or some varient thereof) would be the best solution.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.