It's clear what your code is trying to do, and you have the basic right ideas. There are just a couple of errors/typos and some logic operators you have the wrong way around. Plus, I'll show you a more optimized way to do it in a lot less code.
This is another case where that isn't the line that's
causing the crash. The line where it
is crashing is not normally the line with the error in it. It's because your array index is going past the end of the array, then the next time it hits that line it crashes.
First, you're using "int Position" to access the vector. But you set up a loop iterator which is never used. the loop iterator is declared and is supposedly used as the check for the end of loop. But it's never increased inside the loop. "int Position" is the only thing advanced each loop, but you never check if "Position" has hit the end of the array, so it ends up trying to access outside the array bounds, which causes a crash. The fix there is to either get rid of "int position" and use / advance the iterator, or keep "int Position", get rid of the iterator, and check that Position is within bounds.
while(DoesContain == 0 || CurrentVectorPos != ContainedCoordinates.end())
Here, there should be an "&&" because OR keeps going until BOTH conditions are false. So it would still crash because it would keep trying to read new array values if it never found DoesContain.
if(ContainedCoordinates[Position].x == ax && ContainedCoordinates[Position].y == ay)//<--Line which //causes the crash.
{
DoesContain = 1;
}
else
{
DoesContain = 0;
}
Position++;
you can remove the entire "else" branch, since DoesContain was set to zero before the loop started. You only want to set the DoesContain flag to 1 when found, no need (and actually can be an issue) to set it back to zero if the next one doesn't match.
if(DoesContain == 1)
{
return 1;
}
else
{
return 0;
}
You don't need an 'if' at all, just do 'return doesContain;'
Here's a suggested replacement function:
bool MapSystem::NatureRegion::Contains(int ax, int ay)
{
for(int Position = 0; Position < ContainedCoordinates.size(); Position++)
if(ContainedCoordinates[Position].x == ax && ContainedCoordinates[Position].y == ay)
return 1;
return 0;
}
so, this will basically loop over every valid index, and if it's found will return 1 straight away. If it gets to the end, it will return 0. If you want a loop iterator version, you need to get rid of the Position value altogether, and use the iterator inside the loop to access the data rather than using the [] operator.
It would be something along the lines of ...
bool MapSystem::NatureRegion::Contains(int ax, int ay)
{
for( std::vector <CartesianCoordinate>::iterator CurrentVectorPos = ContainedCoordinates.begin();
CurrentVectorPos != ContainedCoordinates.end(); CurrentVectorPos ++)
if( (*CurrentVectorPos).x == ax && (*CurrentVectorPos).y == ay)
return 1;
return 0;
}
you can do it with for loops or while loops, the logic is the same. I prefer the int index one myself for a vector, it's simpler and faster.
Author
Topic: if self.isCoder(): post() #Programming Thread (Read 909327 times)