🔒 Locked

std::vector and iterators

Started by paic Oct 27, 2005 at 7:07 AM 15 replies 1k views

Original Post

Oct 27, 2005 at 7:07 AM

Hi, I have a piece of code looking like :



std::vector<SSomeStructure>   list;

// ...

for (iter = list.begin(); iter != list.end(); iter++)
{
   bool b = (*iter).ptrToSomeClass->aMethod();
   if (b)
   {
      // then I want to make the current element the first one of the array
      SSomeStructure aStructure = *(list.begin());
      *(list.begin()) = *iter;
      *iter           = aStructure;
   }
}

And it seems to work when I debug step by step. But the second time, I have an exception because my first element is completely screwd up :/ Is it impossible to swap 2 elements using iterators ? (I have solved the problem using [] operator, it works fine. But I'd like to use iterators)

snk_kid

Oct 27, 2005 at 7:26 AM

Quote:
Original post by paic
Is it impossible to swap 2 elements using iterators ?

No, it's not necessary here but you can use std::iter_swap in header algorithm.

Quote:
Original post by paic
But the second time, I have an exception because my first element is completely screwd up :/

Make sure you have properly defined copy semantics if SSomeStructure is the owner or invokes delete ptrToSomeClass in it's destructor. Since the copy constructor and assignment operator implicitly defined does a member-wise (or bitwise if it is a POD-type) copy by default.

gumpy

Oct 27, 2005 at 7:26 AM

Quote:
Original post by paic
Is it impossible to swap 2 elements using iterators ?

#include <algorithm>...std::swap( *iter1, *iter2 );

paic

Oct 27, 2005 at 8:00 AM

The structure really only hold data, so no need for copy constructors (there isn't any method, any allocation, just a few members pointing to external ressources)

But thx, I didn't know there were some swapping functions already existing (I should have guessed -_-)

paic

Oct 27, 2005 at 10:32 AM

Ok, even with the std::iter_swap, I still get the error !
I really don't understand what's going on with iterators >_<

My structure is something like that :

// definitionstruct SStructure{    CFoo   *_pFoo;    char   _name[256];    SStructure()   { _pFoo = NULL; memset(_name, 0, 256);    }};// somewhere else in the code (at initialization time) :SStructure  myStruct;myStruct._pFoo = new CFoo;strncpy(myStruct._name, aCharParameter, 256);myVector.push_back(myStruct);// during runtime, the previous code, replaced by :for (iter = myVector.begin(); iter != myVector.end(); iter++){   if ((*iter)._pFoo->aMethod())   {      std::iter_swap(iter, myVector.begin());      return(true);   }}// and when I quit the app :for (iter = myVector.begin(); iter != myVector.end(); iter++)   delete (*iter)._pFoo;

I really don't understand why the elements of my vectors would be screwd up when I use the std::swap ! When I use myVector instead, I can swap 2 elements without any problem !

I don't want to write all the copy by referance, etc. operators to my structure unless I fully understand why it doesn't work without it. I don't have any method which would touch the _pFoo, nore modify it :/

So, if someone can tell me what's going on with the iterators, I would really appreciate ^^

[Edited by - paic on October 28, 2005 2:32:53 AM]

Enigma

Oct 27, 2005 at 11:13 AM

Well, I can't see anything wrong with your code, other than the fact that _pFoo and _name are not legal identifiers for you to use (they are reserved for compiler implementors), your call to strncpy will not compile (wrong number of parameters) and _name should probably be a std::string. Post the actual code (or preferably a minimal test case that does still show the error behaviour).

Enigma

Nitage

Oct 27, 2005 at 11:38 AM

Quote:
Original post by Enigma
_pFoo and _name are not legal identifiers for you to use (they are reserved for compiler implementors)

No they aren't.

Quote:
From the C++ standard, section 17.4.3.1.2:
Each name that contains a double underscore (__) or begins with an underscore followed by an uppercase letter is reserved to the implementation for any use

EDIT:

Paic, I think you need to add a copy constructor/assignment operator to your SStructure.

paic

Oct 28, 2005 at 2:58 AM

That was me ^^

And I just implemented copy constructors, I still get the error. So here are snapshots of my code, I'm completely clueless on what's going on T_T

// here is the data structure. IMPORTANT : the problem occurs, even if the copy// constructors are not written !struct SDialogEntity{   CDXUTDialog  *_pDialog;   unsigned int _id;   char         _name[256];   SDialogEntity(void)   {      _pDialog   = NULL;      _id        = 0;      memset(_name, 0, 256);   }   SDialogEntity(const SDialogEntity ©)   {      operator = (copy);   }   SDialogEntity	&operator = (const SDialogEntity ©)   {      if(this != ©)      {         _pDialog = copy._pDialog;         strncpy(_name, copy._name, 256);         _id      = copy._id;      }      return(*this);   }};// init time (when the app loads) :SDialogEntity entity;strncpy(entity._name, aNameParameter, 256);entity._pDialog = new CDXUTDialog;m_DialogList.push_back(entity);// runtime : in the message processing function.for (m_DialogIter = m_DialogList.begin(); m_DialogIter != m_DialogList.end(); m_DialogIter++){   if ((*m_DialogIter)._pDialog->MsgProc(hWnd, uMsg, wParam, lParam))   {      // if a dialog box catches an event, we put it at the beginning of      // the list, so that it's drawn on top of all the other ones      if (m_DialogIter != m_DialogList.begin() && (*m_DialogIter)._pDialog->m_bCaption)         std::iter_swap(m_DialogIter, m_DialogList.begin());      return(true);   }}// and when the app close :   for (m_DialogIter = m_DialogList.begin(); m_DialogIter != m_DialogList.end(); m_DialogIter++)      if ((*m_DialogIter)._pDialog)         delete (*m_DialogIter)._pDialog;

When I debug step by step, the first time I do the swap, everything works fine. After the swap, I get iterators to the first element, I parse my entire vector, and everything's fine (no invalide pointer, etc.)

BUT as soon as I leave the message proc function, the next time I come back, it fails, because the _pDialog ptr is invalid, the _name is not initialize, etc.

AND also take into account that when I use m_DialogList (see above post) it works !

I'm completely lost. I wanted to give a try to std::vector, but I think I'll go back to my custom vector implementation which may be a little less optimized, but at least, it works T_T

Enigma

Oct 28, 2005 at 3:57 AM

You might be running foul of iterator invalidation. Do you make any modifications to myVector within aMethod? Beyond that I can't help unless you post more code (i.e. something that can be compiled and demonstrates the problem).

Enigma

paic

Oct 28, 2005 at 4:21 AM

Thx for the reply.
No I don't touch the vector inside the method. It's the message processing method of the dialog box pointed by the _pDialog member of the structures in my vector. It has no visibility of the vector.

And sorry, I can't post a compilable code, unless you want me to send you a mail of a few MBytes ^^ And even if you didn't mind, I can't, it's for my work and there are stuff I can't share.

I'll try to recreate the bug in another program which I will post.

paic

Oct 28, 2005 at 4:41 AM

I can't seem to reproduce the error ... there's defenitely something wrong with my code and it doesn't come from the STL.

So thx everyone for the help, I'll manage (I hope ^^) to find where I made something wrong ^^

mfawcett

Oct 28, 2005 at 11:56 AM

Quote:
Original post by Nitage
Quote:
Original post by Enigma
_pFoo and _name are not legal identifiers for you to use (they are reserved for compiler implementors)

No they aren't.
Quote:
From the C++ standard, section 17.4.3.1.2:
Each name that contains a double underscore (__) or begins with an underscore followed by an uppercase letter is reserved to the implementation for any use

And then there's the following line:

Quote:
— Each name that begins with an underscore is reserved to the implementation for use as a name in the global namespace.

So unless there is some weird interpretation of this, Enigma is correct.

Nitage

Oct 28, 2005 at 12:00 PM

Quote:
Original post by mfawcett
Quote:
Original post by Nitage
Quote:
Original post by Enigma
_pFoo and _name are not legal identifiers for you to use (they are reserved for compiler implementors)

No they aren't.
Quote:
From the C++ standard, section 17.4.3.1.2:
Each name that contains a double underscore (__) or begins with an underscore followed by an uppercase letter is reserved to the implementation for any use

And then there's the following line:
Quote:
— Each name that begins with an underscore is reserved to the implementation for use as a name in the global namespace.

So unless there is some weird interpretation of this, Enigma is correct.

Quote:
— Each name that begins with an underscore is reserved to the implementation for use as a name in the global namespace.

mfawcett

Oct 28, 2005 at 12:13 PM

Quote:
Original post by Nitage
Quote:
— Each name that begins with an underscore is reserved to the implementation for use as a name in the global namespace.

Yes I read that, but that bold font size certainly helped me read it better the second time around.

While you are technically correct, the general consensus is to never use names with a leading underscore. You can still have problems even if you confine your leading underscore named variables to their own namespace, or within classes, and out of the global namespace. Here's an example:

consider how the old (deprecated) version of <ctype.h> worked onmany compilers:extern unsigned short *_pctype;#define isspace(_c) ( _pctype[_c] & _SPACE )#define ispunct(_c) ( _pctype[_c] & _PUNCT )#define isalnum(_c) ( _pctype[_c] & (_UPPER|_LOWER|_DIGIT) )#define isprint(_c) ( _pctype[_c] &(_BLANK|_PUNCT|_UPPER|_LOWER|_DIGIT) )#define isgraph(_c) ( _pctype[_c] & (_PUNCT|_UPPER|_LOWER|_DIGIT) )again, a problem if you have a member variable called _pctype.

And from the boost discussion thread the example was taken from, the OP had this to say:

I am the original poster who started this thread. My questionwas motivated by a problem with the following piece of code:struct IspaceData {  std::map<Miller::Index, int, Miller::hashCompare> IndexDict;  sgtbx::SgOps _sgops;  IspaceData(sgtbx::SgOps& s):_sgops(s){}};With the Compaq Tru64 version 6.2 cxx compiler, this eventuallyresults in unresolved symbols. If "_sgops" is replaced by somethingthat does not start with an underscore, compiling and linkingsucceeds.

Oct 29, 2005 at 5:09 AM

I think you can use _name with 'n' being lowercase but not uppercase. I think I read this in C++ standard in the stl section somewhere. The best is not to use any underscore in global namespace. No problem in your own namespace unless Windows defines some crazy macros that conflict say min/max so I undefine them with NOMINMAX I think and then the std::numeric_limits::max/min() work fine.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.