tags:

views:

281

answers:

5

I have a weird problem. I have a vector that I would like to push objects on to like so:

vector<DEMData>* dems = new vector<DEMData>();
DEMData* demData = new DEMData();
// Build DEMDATA

dems->push_back(*demData);

There will be a few hundred DEMData objects in the vector. The problem is when this code is finished, all items are equal to the last item "pushed back" to the vector?

Why are the other objects being overridden in the vector?

edit:

The DemData class is simple, just a data structure with setters and getters:

    class DEMData
{
private:
    int bitFldPos;
    int bytFldPos;
    const char* byteOrder;
    const char* desS;
    const char* engUnit;
    const char* oTag;
    const char* valType;
    int index;
public:
    void SetIndex(int index);
    int GetIndex() const;
    void SetValType(const char* valType);
    const char* GetValType() const;
    void SetOTag(const char* oTag);
    const char* GetOTag() const;
    void SetEngUnit(const char* engUnit);
    const char* GetEngUnit() const;
    void SetDesS(const char* desS);
    const char* GetDesS() const;
    void SetByteOrder(const char* byteOrder);
    const char* GetByteOrder() const;
    void SetBytFldPos(int bytFldPos);
    int GetBytFldPos() const;
    void SetBitFldPos(int bitFldPos);
    int GetBitFldPos() const;
    friend std::ostream &operator<<(std::ostream &stream, DEMData d);
};

EDIT:

I am reading an XML file and building DEMData objects based on the attributes of each xml element:

DEMData demData;
  for (i = 0; attr[i]; i += 2)
  {
      if(strcmp(attr[i],"BitFldPos") == 0)
      {
      demData.SetBitFldPos(*attr[i + 1] - '0');
      }
      else if(strcmp(attr[i],"BytFldPos") == 0)
      {
        char* pEnd;
        int tmp = strtol(attr[i + 1],&pEnd,10);
        demData.SetBytFldPos(tmp);
      }
      else if(strcmp(attr[i],"ByteOrder") == 0)
      {
        demData.SetByteOrder(attr[i + 1]);
      }
      else if(strcmp(attr[i],"DesS") == 0)
      {
      demData.SetDesS(attr[i + 1]);
      }
      else if(strcmp(attr[i],"EngUnit") == 0)
      {
        demData.SetEngUnit(attr[i + 1]);
      }
      else if(strcmp(attr[i],"OTag") == 0)
      {
        demData.SetOTag(attr[i + 1]);
      }
      else if(strcmp(attr[i],"ValTyp") == 0)
      {
        demData.SetValType(attr[i + 1]);
      }
      else if(strcmp(attr[i],"idx") == 0)
      {
        char* pEnd;
        int tmp = strtol(attr[i + 1],&pEnd,10);
        demData.SetIndex(tmp);
      }
      //printf(" %s='%s'", attr[i], attr[i + 1]);
  }


  // Insert the data in the vector.
  dems.push_back(demData);
A: 

I'm not sure if in this case it's anything to do with the vector itself... The way you're using pointers to the vector and demData (instead of allocating them on the stack) looks a bit suspicious.

I'd expect normal C++ code to look like this:

vector<DEMData>* dems = new vector<DEMData>();
DEMData demData
// Build DEMDATA

dems->push_back(demData);
...
delete dems;

Can you paste the rest of your code? or perhaps the loop that does the demData building?

Orion Edwards
Even then, there's no reason to have `dems` on the heap. Also, take out the parenthesis after `demData`, or you're declaring a function.
GMan
@Orion Edwards: `DEMData demData()` is a function declaration, not an object definition.
AndreyT
woops. Fixed the typo
Orion Edwards
+3  A: 

Please note that your vector holds not references to the objects, but their copies. This means that after you store a newly-created instance of DEMData in your vector, and update the object, the corresponding entry in the vector won't be updated.

You need to make your vector store DEMData*, and push_back a pointer, not a value pointed to.

Vlad
+6  A: 

Why are you allocating the vector with new? Why are you allocating your temporary DEMData object with new?

A vector stores a copy of what you pass to it, not that data itself, so unless you're deleting the DEMData object you allocated with new, you're leaking memory ever time you push an item onto the vector. Likewise, you're setting yourself up for memory leak problems by allocating the vector dynamically.

As to why the objects in the vector all seem to contain the same data, chances are pretty good that you have more of the same -- probably use of pointers combined with an incorrect copy ctor that ends up doing shallow copies a few places it shouldn't -- but since you haven't shown us that code, that's only a guess.

Edit: Now that you've added the code for your DEMData class, it looks very much like the guess above was correct -- you have pointers and no user-defined copy ctor, so you're getting a shallow copy.

As a first step, I'd get rid of all the pointer char members, and replace them with std::strings. The int members should be all right -- copying them will copy the values.

Edit2: Given what you're doing with these member variables, it looks a lot like you should consider using two std::maps -- one for the std::string variables, and one for the int variables.

Jerry Coffin
2Jerry: in which way allocating the vector with new makes a problem here?
Vlad
Perhaps it's not related to the problem at hand, but having a raw pointer hanging around is bad in general. It should be stack-allocated. And if it really needs to be dynamically allocated, it should be wrapped up.
GMan
2GMan: there are indeed some other problems with the code, should we find them all? IMHO concentrating on the actual question at first is a good thing.
Vlad
@Vlad:allocating the vector with new makes the code hard to maintain, hard to understand, and nearly impossible to keep exception safe.
Jerry Coffin
A: 

Your DEMData class needs a copy constructor and destructor in order to manage the strings' memory. As it stands what (probably) is happening is that a DEMData object is created, inserted into the vector which creates a new instance of DEMData with the same pointer values. Then you destroy the original DEMData (since there is no destructor) which leaves the pointers in the vector dangling. Then when you create a new DEMData it gets the same memory locations (by chance) and in the end all the objects point at the same locations.

Motti
+1  A: 

I suppose the strings in the objects are the same. Presumably you use the same demData object to populate the vector.

And since you use default copy constructor all copies contain the same (char*) pointers. Should you change the content of the memory referred by these pointers, all copies 'see' the changes.

shura