need some help with list::end ()

  • Thread starter Thread starter Abubakar
  • Start date Start date
A

Abubakar

Hi all,

I'm using stl's list to store some file names. Its declared as:

list < char * > filenames;
i enumerate the list by using:

list < char * >::const_iterator filename;

filename = filenames.begin();

than I simply do a filename++ to go to the next item.....

I check for filenames.end() in an *if* condition somewhere in my code to
check if I'm dealing with the last item of the list. The code to do this was
working fine. But now due to some change in my code, some of the list items
are deleted *before* the filenames.end() check. I have noticed that this
check now never evaluates to true although I know in the debugger that I'm
standing at the last item. I'm sure I'm missing some basic concept here. How
can I check now that this is the last item of the list?

I'm using vc++ 2k5 total unmanaged.

Regards,

-ab.
 
Abubakar said:
Hi all,

I'm using stl's list to store some file names. Its declared as:

list < char * > filenames;
i enumerate the list by using:

list < char * >::const_iterator filename;

filename = filenames.begin();

than I simply do a filename++ to go to the next item.....

I check for filenames.end() in an *if* condition somewhere in my
code to
check if I'm dealing with the last item of the list. The code to do
this was
working fine. But now due to some change in my code, some of the
list items
are deleted *before* the filenames.end() check. I have noticed that
this
check now never evaluates to true although I know in the debugger
that I'm
standing at the last item. I'm sure I'm missing some basic concept
here. How
can I check now that this is the last item of the list?
Click to expand...

How are you deleting the items? Just doing filenames.erase(filename)
invalidates the iterator to the deleted object.

You should be doing something like

if (some_condition)
filename = filenames.erase(filename);
else
++filename;

This will *either* use the returned iterator to the next object, or
increment the current iterator. You should do one, but not both! :-)


Bo Persson
 
Sorry for the late reply.
How are you deleting the items? Just doing filenames.erase(filename)
invalidates the iterator to the deleted object.
Click to expand...

I use the "remove" method.

I have the following declaration:

list<char * >::const_iterator name, prev;

at some point I do:

prev = name;
name ++;
filenames.remove ( *prev );

and this is how I remove the items.

regards,
-ab.
 
Abubakar said:
Sorry for the late reply.


I use the "remove" method.

I have the following declaration:

list<char * >::const_iterator name, prev;

at some point I do:

prev = name;
name ++;
filenames.remove ( *prev );

and this is how I remove the items.
Click to expand...

If you have duplicates in your list, the call to remove will remove more
than one of them at a time, including possibly the one now referenced by
"name". Use the loop that Bo suggested:

if (some_condition)
filename = filenames.erase(filename);
else
++filename;

that's the canonical way to iterate through a list removing some elements as
you go.

-cd
 
Carl said:
If you have duplicates in your list, the call to remove will remove
more than one of them at a time, including possibly the one now
referenced by "name". Use the loop that Bo suggested:

if (some_condition)
filename = filenames.erase(filename);
else
++filename;

that's the canonical way to iterate through a list removing some
elements as you go.
Click to expand...

Also, how are you adding the items to the list? Since you're storing raw
char*'s, the list isn't managing the memory occupied by the strings - you
are. You should consider changing to a std::list<std::string> instead.

-cd
 
char*'s, the list isn't managing the memory occupied by the strings - you
are.
Click to expand...
list <char * > filenames ;

WIN32_FIND_DATAA finddata;
//emailblockslocation
HANDLE filehandle = FindFirstFileA ( m_folderlocation , & finddata );
char * filename = NULL;
do
{
if ( (finddata.dwFileAttributes & FILE_ATTRIBUTE_ARCHIVE )==
FILE_ATTRIBUTE_ARCHIVE )
{
filename = new char [500];
strcpy ( filename , finddata.cFileName );
filenames.push_back ( filename );

}

} while ( FindNextFileA( filehandle, & finddata ) != 0 );
FindClose ( filehandle );
 
Hi,

Ok I'll try the *remove* method of the list and if I still cant get the
begin() method to work right I'll get back to this post.

Regards,

-ab.
 
Abubakar said:
char*'s, the list isn't managing the memory occupied by the strings
- you are.
Click to expand...
list <char * > filenames ;

WIN32_FIND_DATAA finddata;
//emailblockslocation
HANDLE filehandle = FindFirstFileA ( m_folderlocation , & finddata );
char * filename = NULL;
do
{
if ( (finddata.dwFileAttributes & FILE_ATTRIBUTE_ARCHIVE )==
FILE_ATTRIBUTE_ARCHIVE )
{
filename = new char [500];
Click to expand...

This memory is being leaked when you remove an item from the list - nothing
will call delete[] on it (unless there's yet more code that you haven't
shown that does delete it).
strcpy ( filename , finddata.cFileName );
filenames.push_back ( filename );

}

} while ( FindNextFileA( filehandle, & finddata ) != 0 );
FindClose ( filehandle );
Click to expand...

I'd recommend changing your container to std::list<std::string>, then the
memory for the string will be automatically freed when an item is removed
from the list.

-cd
 
memory for the string will be automatically freed when an item is removed
Click to expand...

yes I was going to write the code for the deletion of char *, but I got
stuck in that filenames.end() condition which btw is still not working. This
std::string idea is good and will reduce my code.

-Ab.

Carl Daniel said:
Abubakar said:
char*'s, the list isn't managing the memory occupied by the strings
- you are.
Click to expand...
list <char * > filenames ;

WIN32_FIND_DATAA finddata;
//emailblockslocation
HANDLE filehandle = FindFirstFileA ( m_folderlocation , & finddata );
char * filename = NULL;
do
{
if ( (finddata.dwFileAttributes & FILE_ATTRIBUTE_ARCHIVE )==
FILE_ATTRIBUTE_ARCHIVE )
{
filename = new char [500];
Click to expand...

This memory is being leaked when you remove an item from the list - nothing
will call delete[] on it (unless there's yet more code that you haven't
shown that does delete it).
strcpy ( filename , finddata.cFileName );
filenames.push_back ( filename );

}

} while ( FindNextFileA( filehandle, & finddata ) != 0 );
FindClose ( filehandle );
Click to expand...

I'd recommend changing your container to std::list<std::string>, then the
memory for the string will be automatically freed when an item is removed
from the list.

-cd
Click to expand...
 
Abubakar said:
yes I was going to write the code for the deletion of char *, but I got
stuck in that filenames.end() condition which btw is still not working.
Click to expand...

Have you tried what Bo and Carl recommended? Here's a complete example:

for(std::list<std::string>::iterator filename = filenames.begin();
filename != filenames.end(); )
{
if(FileNeedsToBeDeletedCondition)
filename = filenames.erase(filename);
else
++filename;
}

If have a radically different mechanism, it's probably not going to work.

I strongly recommend the book Effective STL by Scott Meyers, which
discusses this and dozens of other tricks with STL containers.

Tom
 
Hi,

I have solved the problem by using the list::back() method. As back()
represents the last item in the list, so thats what I was looking for. The
problem with what I was doing was my lack of understanding with the list's
"end" method. I thought that the "end" represents the last item, which is
not correct.

Thanks for the book recommendation!

Regards,

-Ab.

PS: again, sorry for having replied so late,,,, I hate my replying-late
habbit !!!
 
Back
Top