char array corruption in C++

I'm working on a project that makes me store an array of objects whose constructor is

Item(char* item, int itemType){
char temp[200];
    for(int i = 0; i < 200; i++){
        temp[i] = '\0';
        if(item[i] != '\0'){
            temp[i] = item[i];
        }
    }
    _item = item;
    _itemType = itemType;
    _tweetIDs = NULL;
}

Don't worry about _tweetIDs, that's another functional part of my program and isn't related to my problem.

This array is stored within a class:

ItemList()

How this works is that the functional part of my program parses a line of input and puts it into the Item(char*, int) object. This is how it adds the line:

int addItem(char* item, int type){
    char temp1[200];
    for(int i = 0; i < 200; i++){
        temp1[i] = '\0';
    }
    int j = 0;
    while(item[j] != '\0'){
        temp1[j] = item[j];
        j++;
    }
    _items[_size] = Item(temp1, type);
    _size++;
    return _size;
}

Where _items is the Item() array and _size is a field that is incremented every time an Item() is added.

My issue comes when I have to print the contents of the list.

I have a method that does that:

void printList(){

    for(int i = 0; i < 500; i++){
      开发者_如何学Go  if(_items[i] != NULL){
            cout << "[" << i << "] ";
        _items[i]->printContents();
        }
    }
}

I tested printContents() in the constructor of Item() and tested printList in the addItem method and they both work when called within the class itself. The issue comes when I have to call the print method outside the class body.

In the main method, I create a List object:

List itemList;

The default constructor sets all members of the Item() array to NULL and initializes _size.

After adding a few Item() objects into the array (Which I confirmed is increasing in size through the debugger), I tried to print it out. When I call:

itemList.printList();

It gives me the right amount of indexes (And lines), but the char array is just a bunch of garbage. I used the debugger to try and find out where it went wrong. In the addItem() method, I called printList to check the array, and the output from that is fine. Then, I called itemList.printList() right after the last addItem() call, and it gave me garbage. In between the addItem() and itemList.printList(), the char array is lost or something along those lines.

Any idea what's going wrong? I'll give you any more code if you need it.

In your Item constructor, you are setting what I presume is a member _item as such:

_item = item;

This just assigns the pointer value of the location pointed to by item into _item. It does not actually copy the string!

The next time you go to read this location, it might be valid - chances are, though, it will be garbage, as you are seeing.

What you are looking for is a function like strcpy (as a side note, there's no need to do quite so much manual copying - just pass that pointer around and copy it once - in the Item constructor).

EDIT, to address your comment:

strcpy made your program crash because you are using it on unallocated memory.

You have to allocate memory for an array using new[] in c++

Take note on the lifetime of a variable. If you declare temp1 as static array, then it will be destroyed immediately by the end of function addItem.

At the end, all object that refers to this memory location will be invalid.

And .... If you want to pass a reference to an array do it this way:

Item(char** item, int itemType)

I'm imagining your definition of class Item minimally looks like this:

class Item
{
Item(char* item, int itemType);
private:
    char *_item;
};

Your constructor must allocate memory for _item in order to make a copy of what gets passed in via the constructor. Failure to do that will inevitable result in memory problems and exceptions. Alternatively, you can use something like a vector of char.

In Item constructor you create local array char temp[200], you copy there what is pointed by char * item and then you don't use temp[200] any more. What's the point of doing that?

Later you assign passed pointer to _item member. The pointer points to local variable char temp1[200] in addItem(). When addItem() finishes then temp1 is destroyed and so _item in Item class points to garbage.

What you probably need to do is to allocate memory either statically in _item definition or dynamically using new (and then not forget to release it). I think the first solution will be safer for you. In the latter case you would also have to take care of copy constructor and assign operator. So, you need to change _item definition from char * _item to char _item[200], and then you can use strncpy:

Item(char* item, int itemType) {
    strncpy(_item, item, 200);
}