std::vector Question

[Mike Copeland]

In the following code (which seems to work), I must decrement the
vector offset (mmm) when I declare the iterator I use to delete an
object. Can someone explain why, even though I can address the object
(in the for loop) directly?
Or is there a better way to do this? TIA

vector<time_t> scoreTimes;
bool bFound = false;
size_t mmm;
time_t delT1 = [some_value], delT3;
for(mmm = 0; mmm < scoreTimes.size(), bFound == false; mmm++)
{
if(scoreTimes.at(mmm) == delT1) bFound = true;
} // for
if(bFound) // found object to delete
{
vector<time_t>::iterator vIt = scoreTimes.begin()+(--mmm); //???
delT3 = *vIt;
scoreTimes.erase(vIt);
}
}

 

[HungryGoat]

1. The direct method to remove elements from a vector is to use the erase-remove idiom.
scoreTimes.erase(remove(scoreTimes.begin(), scoreTimes.end(), delT1), scoreTimes.end());

2. The condition expression in your for loop uses comma operator which is incorrect. Change it to logical AND operator.
Change this:
for(mmm = 0; mmm < scoreTimes.size(), bFound == false; mmm++)
to:
for(mmm = 0; mmm < scoreTimes.size() && bFound == false; mmm++)

3. The reason why you need to decrement mmm is, in the loop above when the object is found in the vector, mmm is incremented once more and then the condition is checked which then terminates the loop. Eg. If the searched object is at position 3, then the loop is run 4 times and the value of mmm is one more than the position of the object in the vector.

Cheers!

 

[Öö Tiib]

In the following code (which seems to work), I must decrement the
vector offset (mmm) when I declare the iterator I use to delete an
object. Can someone explain why, even though I can address the object
(in the for loop) directly?

vector<time_t> scoreTimes;
bool bFound = false;
size_t mmm;
time_t delT1 = [some_value], delT3;
for(mmm = 0; mmm < scoreTimes.size(), bFound == false; mmm++)
{
if(scoreTimes.at(mmm) == delT1) bFound = true;
} // for
if(bFound) // found object to delete
{
vector<time_t>::iterator vIt = scoreTimes.begin()+(--mmm); //???
delT3 = *vIt;
scoreTimes.erase(vIt);
}

Because for loop works like that. For is:'for(init;condition;update)statement'
You change your 'bFound' in that "statement" but after each "statement" the
"update" is executed that increases your 'mmmm'. You can use 'break' in
"statement" to break out of cycle without "update".

Your "condition" 'mmm < scoreTimes.size(), bFound == false' looks
defective (possibly it is not doing what you want). It feels that you
want && there instead of comma (your case crashes when delT1 is not present
in scoreTimes.

Or is there a better way to do this? TIA

vector<time_t> scoreTimes;
time_t delT3 = 0;

vector<time_t>::iterator vIt;
vIt = std::find( scoreTimes.begin(), scoreTimes.end(), [some_value] );

if ( vIt != scoreTimes.end() )
{
delT3 = *vIt;
scoreTimes.erase( vIt );
}

 

[Öö Tiib]

It feels that you want && there instead of comma (your case crashes
when delT1 is not present in scoreTimes.

I meant your case throws std:ut_of_range that crashes if you do
not catch it outside of posted code.

 

[James Kanze]

In the following code (which seems to work), I must decrement the
vector offset (mmm) when I declare the iterator I use to delete an
object. Can someone explain why, even though I can address the object
(in the for loop) directly?
Or is there a better way to do this? TIA

vector<time_t> scoreTimes;
bool bFound = false;
size_t mmm;
time_t delT1 = [some_value], delT3;
for(mmm = 0; mmm < scoreTimes.size(), bFound == false; mmm++)
{
if(scoreTimes.at(mmm) == delT1) bFound = true;
} // for

The obvious problem here is that you increment mmm one last time
after having set bFound to true. (And while I'm at it: you
never compare a variable with true or false. It should be just
!bFound. And you probably don't want the comma operator there,
either; the compiler should have warned you, but you just throw
away the results of "mmm < scoreTimes.size()", which will cause
problems if delT1 isn't in scoreTimes.)

This is just a linear search. The classic way of writing this
would be something like:

for ( mmm = 0;
mmm != scoreTimes.size() && scoreTimes[mmm] != delT1;
++ mmm ) {
}

And to test whether you've found something:

if ( mmm != scoreTimes.size() )

In modern C++, we'd probably use std::find:

std::vector<time_t>::iterator position
= std::find( scoreTimes.begin(), scoreTimes.end(), delT1 );

and then, if you needed mmm:

ptrdiff_t mmm = position - scoreTimes.begin();

but you probably don't need it, since...

if(bFound) // found object to delete
{
vector<time_t>::iterator vIt = scoreTimes.begin()+(--mmm); //???
delT3 = *vIt;
scoreTimes.erase(vIt);
}

If you've used std::find, above, all you need to do is

if ( position != scoreTimes.end() ) {
delT3 = *position;
scoreTimes.erase( position );
}

 

[James Kanze]

1. The direct method to remove elements from a vector is to
use the erase-remove idiom.

scoreTimes.erase(remove(scoreTimes.begin(), scoreTimes.end(), delT1), scoreTimes.end());

He does something with the position before he erases, so this
won't work. (It also has different semantics: it removes all of
the elements equalt to delT1. His code only removes the first.

2. The condition expression in your for loop uses comma operator which is incorrect. Change it to logical AND operator.

Change this:

for(mmm = 0; mmm < scoreTimes.size(), bFound == false; mmm++)

for(mmm = 0; mmm < scoreTimes.size() && bFound == false; mmm++)

Or better yet, move the comparison which sets bFound up into the
condition. This is, after all, the classical linear search
algorith, so why not write it as such.