CODEWITHSUNDEEP
c++
Blade3
Blade3

Reputation: 4360

C++ vector problem

I have the following class declaration:

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:
    DEMData();
    //DEMData(const DEMData &d);
    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);
    //~DEMData();
};

I am creating a vector to hold objects of the above type like so:

vector<DEMData> dems;

If I were to push_back 100 objects into this vector, then all 100 objects will have the exact same values as the 100th element.

Below is the code snippet:

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);
      }
  }

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

Why would all elements have the same values?

Upvotes: 1

Views: 303

Answers (6)

Mark B
Mark B

Reputation: 96233

This looks exactly like push_back to a Vector to me...is there something different this time?

Assuming that:

  1. The for loop you included is inside another 0-99 for loop
  2. attr[] is maintained somehow between the two for loops and contains either char* or std:strings

The way push_back works is it makes a copy of the data passed in. When you pass a stack object demData all of its attributes are being copied with a default copy constructor which simply copies the actual pointer which (which a simplistic version of the set functions would just pass on through). So even though you set the values into demData each pass through the loop, all you're doing is making 100 items in the vector that are all pointing at the exact same string data: The last data decoded from the source file.

If you posted the code for one of the functions like SetEngUnit that would help confirm this.

Options are:

  1. Make the char* in the class std::string, which should manage all the memory for you.
  2. Implement the copy constructor and operator= for your class to do a deep copy.
  3. Make sure when you call the set methods that you pass in a distinct pointer each time. You'll need to make sure you clean it up right or you leak memory.

Upvotes: 0

fogo
fogo

Reputation: 304

You should make sure you variable attr is being properly modified in every iteration. Your snippet doesn´t make it clear, but you are always reading the same range from attr. If by any chance you forgot to change this variable for each iteration, you will end up with the same elements in your vector everytime.

Upvotes: 0

JoMo
JoMo

Reputation: 436

Make sure you are creating a new value for the DEMData inside the loop. If you do this : 

DEMData dem;

for( int i = 0; i < 100 ; i++ )
{
    modifyDem( dem, i );
    vec.push_back(dem);
}

The vector will only contain the information of the last one modified (because the memory of the object gets overwritten every time).

If you do this : 

for( int i = 0; i < 100 ; i++ )
{
    DEMData dem;

    modifyDem( dem, i );
    vec.push_back(dem);
}

You should be o.k.

Upvotes: 0

sth
sth

Reputation: 229563

You seem to call push_back after the end of the for-loop, so that only the last element will be stored in the vector. Is that your intention?

Also the objects might directly store the char* from the attr array, so make sure that the memory used there stays valid and is not overwritten/freed later on.

Upvotes: 1

EightyEight
EightyEight

Reputation: 3460

push_back() makes a copy internally, but your DEMData class has no copy constructor, so all of the elements in the vector end up pointing to the same data. When you modify one, you modify all. Since the last element is modified last, you get all of the previous elements mirroring the last one.

HTH

Upvotes: 1

Liz Albin
Liz Albin

Reputation: 1489

Are you doing the push_back in the loop? If not, demData won't change.

If your push_back is within the loop, the values of demData are dependent on the attr[i] values. If those don't change, members of demData won't change.

Upvotes: 4

Related Questions