std::vector Question

Discussion in 'C++' started by Mike Copeland, Jul 3, 2013.

  1. 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);
    }
    }
     
    Mike Copeland, Jul 3, 2013
    #1
    1. Advertising

  2. Mike Copeland

    HungryGoat Guest

    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!
     
    HungryGoat, Jul 3, 2013
    #2
    1. Advertising

  3. Mike Copeland

    Öö Tiib Guest

    On Wednesday, 3 July 2013 07:05:13 UTC+3, Mike Copeland wrote:
    > 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, Jul 3, 2013
    #3
  4. Mike Copeland

    Öö Tiib Guest

    On Wednesday, 3 July 2013 07:45:10 UTC+3, Öö Tiib wrote:
    > 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::eek:ut_of_range that crashes if you do
    not catch it outside of posted code.
     
    Öö Tiib, Jul 3, 2013
    #4
  5. Mike Copeland

    James Kanze Guest

    On Wednesday, 3 July 2013 05:05:13 UTC+1, Mike Copeland wrote:
    > 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
     
    James Kanze, Jul 3, 2013
    #5
  6. Mike Copeland

    James Kanze Guest

    On Wednesday, 3 July 2013 05:40:51 UTC+1, HungryGoat wrote:
    > 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++)


    > to:


    > 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.

    --
    James
     
    James Kanze, Jul 3, 2013
    #6
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Peter Jansson
    Replies:
    5
    Views:
    6,423
    Ivan Vecerina
    Mar 17, 2005
  2. Anonymous
    Replies:
    20
    Views:
    4,427
    Pete Becker
    Mar 30, 2005
  3. Jason Heyes
    Replies:
    8
    Views:
    767
    Andrew Koenig
    Jan 15, 2006
  4. Replies:
    8
    Views:
    1,996
    Csaba
    Feb 18, 2006
  5. Rune Allnor
    Replies:
    4
    Views:
    992
    Rune Allnor
    Dec 11, 2008
Loading...

Share This Page