Scrambled pointers

[kamikazeUnicorn]

Hi All,

I'm really struggling to get a list of pointers to objects to work properly. I have two global lists, vector<Field*> input; vector<Field*> output;, which contain objects of a subclass of Field, either polarField or metricField. In a third global list, I then want to contain alternating pointers to objects in each list vector<Field**> links;. Unfortunately, when I try to print out elements from the third list, all the properties of the fields are the extremes of the data types.
I have tried using a single * in the third list, and this does resolve the printing problem, however any attempts to resize the list, or pass it around result in a null list.
Below are what I think are the relevant extracts of my code, related to the use of the third list:

// in the header file:
std::vector<Field**> links;
std::vector<Field**> getLinkedFields() const{return linkedFields;}

// in the cpp file:
void init(){
   ...
   links.reserve(linkCapacity);
   ...
}

bool FieldFieldMapping:: addLink(Field *inputField, Field *outputField)
{
   if((numLinks*2+2)>linkCapacity)
   {
      //expand list
      links.reserve(linkCapacity+2);
      linkCapacity += 2;
   }
   links[numLinks*2] = &inputField;
   links[(numLinks*2)+1] = &outputField;
   numLinks++;
}

bool FieldFieldMapping::deleteLink(Field *inputField, Field *outputField)
{
   for(unsigned int i=0; i<numLinks; i++)
   {
      if(**links[i*2] == *inputField && **links[i*2+1]==*outputField)
      {  //don't want to delete the actual object by calling erase on vector
         for(int j=i*2; j<numLinks*2-2; j++)
         {
            links[j] = links[j+2];
         }
         links.resize(numLinks*2-2);
         numLinks --;
         linkCapacity = numLinks*2;
         return true;
      }
   }
   return false;
}

void FieldFieldMapping:: printLinkedFields()
{
   for(size_t i=0; i<numLinks; i++)
   {
      cout << "There is a link between ";
      Field* inputField = *links[i*2];
      Field* outputField = *links[i*2+1];
      switch (inputFieldType)
      {
         case POLAR_FIELD:
         {
            polarField* pfield = (polarField*)inputField;
            cout << "polar field: ID: "<< pfield->getIndex() << " " << *pfield << endl;
            break;
         }
         ...
      }
   }
}

Any help, or explanation of what is happening would be greatly appreciated.

[Disch]

You have quite a few misunderstandings:

1) You don't need to check the bounds of whether or not you need to expand vector. vector does that automatically, that's kind of the whole point of using it.

2) reserve doesn't expand the vector anyway. You probably meant resize. Although I wouldn't use resize for this either, I'd probably use push_back

3) Using a Field** seems rather silly, and makes the code rather cumbersome (I'm getting lost in all your indirections). Just use a Field*

4) //don't want to delete the actual object by calling erase on vector This is an incorrect assumption. erase() will remove the pointer from the vector. It will not delete what the pointer points to.


#2 is really a big problem. It's probably why you're getting all screwed up. Anyway this code can be greatly simplified

Also... what is linkedFields? Is that different from links? And wouldn't putting links in the header give you linker errors? (globals are evil, globals in headers are especially evil)

std::vector<Field*> links;   //  go back to Field*


// since this doesn't return anything, I changed to void
void FieldFieldMapping:: addLink(Field *inputField, Field *outputField)
{
   // just add the Field*s to the vector.  Let vector take care of the resizing.
   links.push_back(inputField);
   links.push_back(outputField);

   ++numLinks;
}


bool FieldFieldMapping::deleteLink(Field *inputField, Field *outputField)
{
   for(unsigned int i=0; i < links.size(); i += 2)  // IMO this is more clear
   {
      // no need to do any dereferencing here.  You probably just want to compare the pointers
      if(links[i] == inputField && links[i+1] == outputField)
      {
         // just erase the links
         links.erase( links.begin() + i, links.begin() + i + 2 );
         --numLinks;
         return true;
      }
   }
   return false;
}

Although there are probably even better ways to go about this. Like maybe using a map instead of a vector, and putting both parts of the link in a struct instead of having 2 entries per link.

[helios]

//don't want to delete the actual object by calling erase on vector

erase() doesn't follow pointers. Likewise, destructing a vector of pointers doesn't free the objects pointed to by the pointers in the vector.

links would work better as an std::list<Field *>. I don't see anything that could cause you described (about getting a null list when resizing it or passing it around).

[kamikazeUnicorn]

Thank you Disch, that new code works. I don't know what I was doing wrong when I was trying to use just Field* with erase() and the other approach I had, but I was definitely getting some strange errors. The linkedFields was a typo, and should have been links. Also, my terminology was slightly wrong with the global variable, I should have said it was a private member variable of the class, so hopefully not quite so evil.
As the list is now working, I will leave it as it is, but in the future I may try switching it over to using a struct to contain the two entries, as a neater solution.

Helios: thank you for the correction. After reading the API for vector, I was worried that it would follow the pointer and delete the object, especially when I was getting strange errors at this point, but seems I was wrong.

Thanks for all your help! Now I can finally start to use this code.