At a loss with vector<string> search...

Started by
5 comments, last by flamewill 20 years, 3 months ago
I''m speechless. Is there any difference between the two code snippets below? They are both supposed to do the same thing: search a string vector (ie. neighbours) for a match (ie. destination). Code Snippet One works fine, while Code Snippet Two crashes. My bug hunt in Code Snippet Two so far tells me the problem lies in the while loop. Visual Studio 6.0 is used in this case.

// Snippet One

vector<string> neighbours = pTerritory->getNeighbours();
vector<string>::iterator iter = neighbours.begin();

while( iter!=neighbours.end() )
{
	if( *iter==destination )
	{
		pGame->getWorld()->updateResidence( _pActor->getName(), destination );

		return;
	}

	iter++;
}

// Snippet Two

vector<string> neighbours = pTerritory->getNeighbours();
vector<string>::iterator iter = neighbours.begin();

while( ( destination!=(*iter) )&&( iter!=neighbours.end() ) )
{
	iter++;
}

if( destination==(*iter) )
{
	pGame->getWorld()->updateResidence( _pActor->getName(), destination );
}
Blt 'em Blittin' Blips.
Advertisement
In the second while loop you should swap the order of ( destination!=(*iter) )&&( iter!=neighbours.end() ). Check to see if iter != neighbours.end() first. Otherwise you''re derferencing bad data.
Try switching the positions of the conditions

while( ( iter!=neighbours.end() ) && ( destination!=(*iter) ) )

Peace Out!
As said above, but also the last test

if( destination==(*iter) )

is wrong if iter is at end()
Just a quick reminder.
With iterator, always use pre-increment ( ++iter ).
Ok, for POD the compiler should optimize it (I know, this has been a sudjet for thousands of threads on this forum), but I''m not sure about operator overloading.

And instead of "if( destination==(*iter) )", simply check iter against neighbours.end( ). It''s faster and handles the case where iter is the end.

----------------
Blaster
Computer game programmer and part time human being
Strategy First - http://www.strategyfirst.com
Blastersoft - http://www.blastersoft.com
As a side note, be aware that your first line is copying what could potentially be great deal of data. A good way to fix that issue is to make sure that getNeighbours() is defined something like:

const vector<string>& Territory::getNeighbors();

and the first two lines would be changed to

const vector<string>& & neighbors = pTerritory->getNeighbors();
vector<string>& ::const_iterator iter = neighbors.begin();

That way you'll be copying very very little data, since you're only using references, and saving a lot of processor time (if that matters here).

[edited by - Agony on January 14, 2004 12:32:05 PM]
"We should have a great fewer disputes in the world if words were taken for what they are, the signs of our ideas only, and not for things themselves." - John Locke
Sweet heaven of mercy! I thought it was weird to crash in a loop with only one statement!

That certainly fixed the problem, people. The const. reference and pre-increment ideas are also useful. Thank you all! :D
Blt 'em Blittin' Blips.

This topic is closed to new replies.

Advertisement