Iterating though a vector: Pointers?

Started by
5 comments, last by Zahlman 12 years, 11 months ago
Hi, I'm new to game programming so this might just be completely the wrong way to go about this. I'm trying to read from two files, names.txt and scores.txt which record the names of the people on the high-scores table and their scores respectively and insert the new name and score so the scores are increasing on the high-score table

char broke = 'n';

for (int i = 0; i < score_string.size(); i++)
{
vector::iterator it = score_string.begin() + i;

if (atoi(score_string.c_str()) <= new_score)
{
name_string.insert(it, player);
score_string.insert(it, convert_int(new_score));
broke = 'y';
break;
}
}




Where 'broke' just records the fact that the loop was exited early for use later on, name_string and score_string are vectors containing the names and scores respectively and new_score is the integer value representing the score of the current player. Both name_string and score_string are the same length by default and whenever I modify one, I modify the other, but for some reason I keep getting an error saying the vector iterator is outside of the range.

The rest of my code seems to work fine, it's just trying to insert the new score that causes problems. Am I supposed to use pointers with the iterator? If so could you explain how?

Thanks,
Pixel
Advertisement
Presumably the vector is empty at first. begin()+1 will therefore return an invalid iterator as begin()==end() on an empty vector. You are also trying to use an iterator from score_string on name_string, which is very bad mojo indeed.

Why are you inserting? Why not just push_back()?


name_string.push_back(player);
score_string.push_back(convert_int(new_score));


HTH
It is invalid to try to use a iterator on a different vector ("it" belonds to "score_string" so cannot be used with "name_string").

Instead of maintaining two parallel arrays, use a single vector of data structures:

struct HighScore
{
std::string name;
int score;
};

// Later

std::vector<HighScore> scores;


Split your function in two: part the finds the correct location to insert and the part that inserts.

You could be lazy and just push_back the value and then sort the array using std::sort(). Alternatively, if you can guarantee the array is always sorted you could use something like lower_bound() to find the correct location to insert the new object. Or you could do as you currently are and hand write a loop to find the correct location.

Presumably the vector is empty at first. begin()+1 will therefore return an invalid iterator as begin()==end() on an empty vector. You are also trying to use an iterator from score_string on name_string, which is very bad mojo indeed.

Why are you inserting? Why not just push_back()?


name_string.push_back(player);
score_string.push_back(convert_int(new_score));


HTH


I guess I should have mentioned that sorry. For the purposes of testing I added names and scores to the text files already and I think if I were to implement this to some kind of game, I'd put in one or two entries (similar to those old computer games where the high-score list was filled with a bunch of names and scores even before you'd played it).

In my code I start off with i = 0 when I loop through score_string, so technically I shouldn't have that problem anyway, no?

And when it comes to using the same iterator in both vectors, they're always the same length, one just represents the scores of the people in the other. That can't cause a problem can it?
Ok, thanks to both of you, I think I got it now smile.gif

And when it comes to using the same iterator in both vectors, they're always the same length, one just represents the scores of the people in the other. That can't cause a problem can it?


Mother of god, yes. An iterator is nothing like an index. In fact, it is quite permissible by the standard to implement a std::vector iterator as a raw pointer. Few implementations actually do, but you need to treat them as if they are.

Don't ever try to use an iterator on any other container than the one you got it from. Simple rule, no exceptions.
1) As noted, you may not use an iterator from one container with another container.
2) As noted, you should have a structure for your data. Group related data.
3) As noted, iterators are not indexes. So you also don't update them like indexes in order to iterate. (Your current attempted use of iterators gains you exactly nothing.) You iterate by updating the iterator object itself in the loop.
4) You don't need an explicit 'broke' flag. All you do is scope the iterator outside the loop, and then check if it actually reached the end.
5) You apparently have a function called 'convert_int' which converts a string to the corresponding int. You should also be able to convert the other way around. You should not use primitive tools like atoi here. Strongly consider boost::lexical_cast. BUT keep data in a logical format. Convert scores to int as soon as you load them from disk, and keep them that way. Don't convert them until it's time to write them to disk. Note that the conversion will then be automatic: you just use [font="Courier New"]operator<<[/font] and [font="Courier New"]operator>>[/font] appropriately.
6) Break the task down: first figure out where to insert the score, then insert it.
7) Actually, you don't need to write a loop at all, because the standard library provides an algorithm for the "figure out where to insert the score" logic. What we do is write a test for "is this the appropriate place to insert the score?" and then feed it to an algorithm.


struct HighScore {
int score;
string name;
}

// We define a comparison for HighScores, by comparing the score.
bool operator<(const HighScore& a, const HighScore& b) {
return a.score < b.score;
}

void update(vector<HighScore>& scores, HighScore& new_score) {
// Figure out where to insert the score. I assume you put the highest score at the beginning.
// We want the first position where we can insert the value without violating the ordering,
// which we get with std::lower_bound. However, these algorithms assume ascending order
// by default, and we have descending order, so we have to tell it about the ordering.
// Then we will insert the new score at that point, and remove the last element. This works
// even if the score isn't a new high score, since we'll just append and then remove it. The
// result of the lower_bound calculation goes directly to the insertion operation.
scores.insert(std::lower_bound(scores.begin(), scores.end(), new_score, std::greater), new_score);
scores.pop_back();
// So after all that discussion, it's actually very simple. We don't even really need a comment here.
}

This topic is closed to new replies.

Advertisement