CODEWITHSUNDEEP
c++
Ian Fiddes
Ian Fiddes

Reputation: 3011

Initializing Dynamic Array of Strings (C++)

I am working on assignment to create a container class for a dynamic array of strings. I know that it would be much easier/better done with std::vector, but that is not the point. I am having a problem finding the right way to initialize my array in the constructor. The way it is below, I am still being warned by the compiler that the variable lineArray is not used. The program compiles with a warning that lineArray is unused then hangs at runtime.

MyBag::MyBag()
{
    nLines = 0;
    std::string lineArray = new std::string[0] ();
}
void MyBag::ResizeArray(int newLength)
{
    std::string *newArray = new std::string[newLength];
    //create new array with new length
    for (int nIndex=0; nIndex < nLines; nIndex++)
    {
        newArray[nIndex] = lineArray[nIndex];
        //copy the old array into the new array
    }
    delete[] lineArray; //delete the old array
    lineArray = newArray; //point the old array to the new array
    nLines = newLength; //set new array size
}
void MyBag::add(std::string line)
{
    ResizeArray(nLines+1); //add one to the array size
    lineArray[nLines] = line; //add the new line to the now extended array
    nLines++;
}

http://ideone.com/pxX18m

Upvotes: 1

Views: 4741

Answers (4)

jxh
jxh

Reputation: 70502

In addition to the shadowed member variable, and the ResizeArray to smaller array issue, there is a bug in your add() method, as indicated by 6602. After your call to ResizeArray, nLines has already been updated to the new value, so you are actually writing to the wrong array position, and then wrongly incrementing nLines again. Make sure to write to the correct position, and there is no need to increment.

void MyBag::add(std::string line)
{
    int oldLength = nLines;
    ResizeArray(nLines+1); //add one to the array size
    lineArray[oldLength] = line; //add the new line to the now extended array
}

Upvotes: 1

stardust
stardust

Reputation: 5998

Warning to the rescue. Good thing that you had compiler warnings otherwise this would have been a bug which will have taken longer to figure out. 

std::string lineArray = new std::string[0] ();
^^^^^^^^^^^

is declaring a new variable called lineArray with in the constructor. You are not using the class member one. The member lineArray pointer will still be pointing to some uninitialized memory.

It should be

lineArray = new std::string[0] ();

Upvotes: 1

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 727137

In addition to the obvious error reported by compiler (i.e. initializing a local variable rather than assigning to an instance variable) you have a more serious issue: if a value smaller than nLines is passed to ResizeArray, your code would exhibit undefined behavior by writing data past the end of the allocated region. You need to change the code as follows:

void MyBag::ResizeArray(int newLength)
{
    // Add a trivial optimization:
    if (newLength == nLines) {
        // No need to resize - the desired size is already set
        return;
    }
    std::string *newArray = new std::string[newLength];
    //create new array with new length
    for (int nIndex=0; nIndex < nLines && nIndex < newLength ; nIndex++)
    {   //                             ^^^^^^^^^^^^^^^^^^^^^
        newArray[nIndex] = lineArray[nIndex];
        //copy the old array into the new array
    }
    delete[] lineArray; //delete the old array
    lineArray = newArray; //point the old array to the new array
    nLines = newLength; //set new array size
}

Upvotes: 1

juanchopanza
juanchopanza

Reputation: 227618

You are using a local variable called lineArray in your constructor. You want to use your data member, for example:

MyBag::MyBag()
{
    nLines = 0;
    lineArray = new std::string[0] ();
}

Upvotes: 1

Related Questions