Hello,
[EMAIL PROTECTED] wrote:
> void check_jobs()
> {
> set<timer_job*>::iterator i, deletion_candidate;
> bool was_first;
>
> for (i = jobs.begin(); i != jobs.end(); i++)
> {
> // Check each job in our queue/set,
> // to see if by any chance it has timed out
>
> deletion_candidate = i;
> if (i == jobs.begin()) was_first = true;
> else was_first = false;
>
> if ((*i)->check() > 0) // if job has timed out
> {
> // do not decrement past the array base
> if (! was_first) i--;
>
> delete (*deletion_candidate); // kick off, you
> jobs.erase(deletion_candidate);
>
> // reset ptr to the new array base
> if (was_first) i = jobs.begin();
> // else OK, go ahead from the previous element,
> // incrementing at loop end
> }
> // else OK, this job is still pending
After erasing the last element, i will be equal to jobs.end() and then
i++ in the head of the for loop.... You have so many quirks built into
this function that it should have made you suspicious. Something that
simple cannot be so complicated to program.
> }
> }
You can't use a for loop, because after erase the iterator to be
incremented is no more, and you cannot avoid the increment of the for
loop.
iterator i = ...begin()
while (i != ...end()) {
iterator candidate = i++;
if ( do_remove(candidate)) {
delete *candidate;
...erase(candidate)
}
}
But then use remove_if and smart pointers to make your code a lot less
ugly. Could be basically a one-liner.
Bernd Strieder
_______________________________________________
Help-gplusplus mailing list
Help-gplusplus@gnu.org
http://lists.gnu.org/mailman/listinfo/help-gplusplus
