Quick vector question

Um, two things first: Could you use code tags, and that isn't even compilable code in the first place(??).

-Aryoc

[edited by - Aryoc on June 1, 2004 7:20:23 PM]

bool StoreItem(item itemObj){

	if(Inventory.size()==Max_Inventory)	
		return false;

	Inventory.push_back(itemObj);
	return true;
}

Just an observation, but I don't believe you are allowed to have a variable/parameter name set as "item" when "item" is already defined as a class/struct. Try calling it itemObj or something instead.

Edit: Stuffed up my tags

[edited by - wavarian on June 1, 2004 10:24:23 PM]

[edited by - wavarian on June 1, 2004 10:25:45 PM]

First, use:
#include <vector>
#include <iostream>
#include <conio>

instead of:
#include <vector.h>
#include <iostream.h>
#include <conio.h>

The latter are not maintained anymore. (I believe some STL libraries not even contain them anymore ?)

Anyway the reason why the program is not giving you the output you are exepcting is because this line:
vector Inventory(5);
fills your newly created Inventory vector with 5 uninitialized items.
(at positions 0...4 )

So when you add your sword to the inventory it gets stored at position 5.
try this:
cout << Inventory[5].name;

or if you want the last item added to the Inventory:
cout << Inventory.back().name;

post using {source} {/source} tags replacing { with [. You can go and edit your previous posts to make them readable.

push_back will increase the size of the vector.

your items will be default constructed because you haven''t written one yourself. this means the integers will be uninitialised (the string will construct itself correctly as it has a default constructor defined). make a constructor and initialise the values to some default. then you might have more success printing them out.

show us the exact code that is making it crash. use source tags. if you don''t know how, read the faq at the top of the page.

vector Inventory(5);

why are you declaring your std::vector like this?

The advantage to this vector class is that you can dynamically add elements and not worry about defining a size.

vector<item> Inventory;

Thats how I would declare it, that way when you push_back(item) you wont get access violations.

______________________________________

what we do in life, echoes in eternity


[edited by - Lokken on June 2, 2004 8:41:52 AM]

Your Item class has a string object... And when you make a push_back or you pass the object to a function it uses it''s copy constructor... When there is none it just makes a copy like when you use a structures ... this means bit to bit copy... Which is very bad when you use std::string.

In fact by copying a std::string bit per bit you do not copy the char array that is behind, but the pointer that is pointing to that array. This means that your copied object and your original object have each a pointer pointing to the same memory address... And when your copied object is beening destroyed when you exit your function, it releases the memory allocated by the other std::string...

The only solution is to make the operator= and copy constructor for this class like here :

class item { 
  public:
   int strength;
   string name;
   int effect;

   item& operator= (const item& inItem ) {
     strength = inItem .strength;
     name     = inItem .name;
     effect   = inItem .effect;
   }

   item (const item& inItem ) {
     strength = inItem .strength;
     name     = inItem .name;
     effect   = inItem .effect;
   }


};

You call an explicit operator = on std::string which does all the right work for you.

If you need more precisions, I might be able to help you... just ask.

#include <iostream>
#include <vector>
#include <string>

class Item{
public:
    Item() : m_strength(0), m_effect(0) {
    }
    Item(int strength, const std::string& name, int effect) :
        m_strength(strength), m_name(name), m_effect(effect) {
    }
    int m_strength;
    std::string m_name;
    int m_effect;
};

typedef std::vector<item> Items;
Items inventory;
const int Max_Inventory = 99;

bool StoreItem(const Item& item) //returns false if inventory is full

{
    if(inventory.size()==Max_Inventory)
        return false;

    inventory.push_back(item);
    return true;
}

int main()
{
    Item sword(1, "sword", 0); 
    Item armor(1, "armour", 0);

    std::cout << StoreItem(sword);
    std::cout << inventory[0].name;

    int i;
    std::cin>>i;
    return 0;
}

quote:

---

Original post by werbfred
Your Item class has a string object... And when you make a push_back or you pass the object to a function it uses it''s copy constructor... When there is none it just makes a copy like when you use a structures ... this means bit to bit copy...

---

nonsense.

the structure will automatically copy the members using their operator=. for built-in types that''s fine. for std::string that''s fine too.

if you just had a pointer you were copying though that could lead to the problems you are worrying about. as it is, nothing to fear.

quote:

---

Original post by werbfred
If it''s non sense check the pointer you are pointing too in both objects and you will see that they both point to the same memory... It''s so much non sense that in my developpement team we already experienced the same problem...

---

There is no pointer in the item structure.

class item
{
public:
    int strength;
    string name;
    int effect;
};

"string name;" is a object.
This means that when an object of class "item" gets copied, either through its copy constructor or assignment operator, it will call the copy constructor or assignment operator of the string as well.

The string object will then create a copy of its underlying buffer. So that when one string gets deleted the other still has a valid buffer.

Try this:

#include <iostream>
#include <string>

using namespace std;

class Foo
{
public:
    string s;
};

#pragma argsused
int main(int argc, char* argv[])
{
    Foo *a = new Foo;
    a->s = "i''m a happy string";

    // Copy using copy constructor

    Foo b(*a);

    // Copy using asignment

    Foo c;
    c = *a;

    // delete a

    delete a;

    cout << b.s << endl;
    cout << c.s << endl;

    char blah;
    cin >> blah;
    return 0;

}

Ok, you''re right after checking... our problem occured while setting a typedef before the declaration... and no there''s nothing similar in both cases...

Appologies...

quote:

---

Original post by werbfred
Ok, you''re right after checking... our problem occured while setting a typedef before the declaration... and no there''s nothing similar in both cases...

Appologies...

---

easy mistake to make.

might be a good idea to delete (or modify) your original post though

no problem

After commenting out #include <conio.h> (which doesn''t exist here), your code compiles (g++ 3.3.2) and runs fine on my machine.

---

“Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it.” — Brian W. Kernighan

cmon someones gotta have an answer Why is devc++ not compiling this code correctly?

try this, i''m not sure that you can serialize a std::string:
cout << Inventory.back().name.c_str();

-me