std::list safely pop out iterator

T

TBass

Hi,

I've got a class that uses a std::list to store tags that need to be
written out to a device. I've had no problem with the lists in terms
of iterating and reading values. My problem is when I need to pop that
value off the list when I'm done with it.

In this case, I think the iterator is getting "lost" because I pop off
the value it is pointing to at the end of this function. But how do I
pop values off one at a time without the iterator getting lost? The
erase function will just have the same problem.

Instead of hacking at it, I was wondering if anyone could point me
towards the standard solution when dealing with std::list.

Thanks in advance,
T

unsigned int duty_count;
std::list<CFusionTag *> *mylist;
std::list<CFusionTag *>::iterator it;
FU_Tag fu_tag;
FusionPlugin *myplg = m_pDevice->Driver();
CFusionTag *mytag;

mylist = m_pDevice->GetWriteTagList();
for( it=mylist->begin(), duty_count = 0;
it != mylist->end() && duty_count < m_pDevice->DutyCycle();
++it, ++duty_count )
{

mytag = *it;

if ( mytag->IsRegistered() )
{

if ( mytag->ClientAccess() == mytag->AccessWriteOnly
|| mytag->ClientAccess() == mytag->AccessReadWrite )
{


memcpy( fu_tag.Address, mytag->Address(), strlen( mytag-
Address() ) );
fu_tag.DeviceID = m_pDevice->RKey();
fu_tag.TagID = mytag->RKey();
fu_tag.TagType = mytag->Type();
mytag->CreateFUTagValue( &fu_tag.Value );
mytag->GetValue( &fu_tag.Value );

int nret = myplg->m_PluginModules->SetTagValue( &fu_tag );
if ( nret <= 0 )
{
mytag->Quality( 0 );
}

/* DELETE THE VALUE POINTER WE CREATED */
delete fu_tag.Value;

/* POP THIS OFF THE LIST */
mylist->pop_front();
}

}

}
 
J

Jim Langston

TBass said:
Hi,

I've got a class that uses a std::list to store tags that need to be
written out to a device. I've had no problem with the lists in terms
of iterating and reading values. My problem is when I need to pop that
value off the list when I'm done with it.

In this case, I think the iterator is getting "lost" because I pop off
the value it is pointing to at the end of this function. But how do I
pop values off one at a time without the iterator getting lost? The
erase function will just have the same problem.

Instead of hacking at it, I was wondering if anyone could point me
towards the standard solution when dealing with std::list.

Thanks in advance,
T

unsigned int duty_count;
std::list<CFusionTag *> *mylist;
std::list<CFusionTag *>::iterator it;
FU_Tag fu_tag;
FusionPlugin *myplg = m_pDevice->Driver();
CFusionTag *mytag;

mylist = m_pDevice->GetWriteTagList();
for( it=mylist->begin(), duty_count = 0;
it != mylist->end() && duty_count < m_pDevice->DutyCycle();
++it, ++duty_count )
{

mytag = *it;

if ( mytag->IsRegistered() )
{

if ( mytag->ClientAccess() == mytag->AccessWriteOnly
{


memcpy( fu_tag.Address, mytag->Address(), strlen( mytag-
fu_tag.DeviceID = m_pDevice->RKey();
fu_tag.TagID = mytag->RKey();
fu_tag.TagType = mytag->Type();
mytag->CreateFUTagValue( &fu_tag.Value );
mytag->GetValue( &fu_tag.Value );

int nret = myplg->m_PluginModules->SetTagValue( &fu_tag );
if ( nret <= 0 )
{
mytag->Quality( 0 );
}

/* DELETE THE VALUE POINTER WE CREATED */
delete fu_tag.Value;

/* POP THIS OFF THE LIST */
mylist->pop_front();
}

}

}

Okay, you are using pop_front() to delete an element. How do you know that
it is pointing to the first element though? You are incrementing it in the
for loop, it may not be pointing to the first element. Wouldn't it be
better to do mylist->erase( it )? And erase returns an element to the next
item in the list. The normal algorithm for this is:

for ( std::list<CFusionTag *>::iterator it = MyList.being(); it !=
MyList.end(); }
{
if ( somecondition )
it = MyList.erase( it );
else
++it;
}

Notice that it is not being incremented in the for loop. It is only
incremented if the element is not erased.

Now, if you really are wanting to erase the front elements only (which seems
like a bug in your code, but I may be wrong) then simply have it point to
the first element again after erasing it.
it = MyList->begin();
 
D

Daniel T.

TBass said:
Hi,

I've got a class that uses a std::list to store tags that need to be
written out to a device. I've had no problem with the lists in terms
of iterating and reading values. My problem is when I need to pop that
value off the list when I'm done with it.

In this case, I think the iterator is getting "lost" because I pop off
the value it is pointing to at the end of this function. But how do I
pop values off one at a time without the iterator getting lost? The
erase function will just have the same problem.

Instead of hacking at it, I was wondering if anyone could point me
towards the standard solution when dealing with std::list.

I edited your code for brevity. As Jom Langston already noted, poping
the first object off of the list is not the correct thing to do.
for( it=mylist->begin(); it != mylist->end(); ++it )
{

mytag = *it;

if ( mytag->IsRegistered() )
{

if ( mytag->ClientAccess() == mytag->AccessWriteOnly
|| mytag->ClientAccess() == mytag->AccessReadWrite )
{

mylist->pop_front();
}
}

}

The idiomatic way to do this would be something like:

it = mylist.begin();
while ( it != mylist.end() )
{
if ( /* some condition */ )
it = mylist.erase( it );
else
++it;
}
 
T

TBass

      it = mylist.erase( it );

Ah. I did not know erase returned an iterator. Thank you both very
much.

Thanks!
 
D

Daniel T.

TBass said:
Ah. I did not know erase returned an iterator. Thank you both very
much.

In the specific case of list, you don't need it to.

mylist.erase( it++ );

works too, but using the return is more generic.
 

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,768
Messages
2,569,574
Members
45,048
Latest member
verona

Latest Threads

Top