CODEWITHSUNDEEP
c++arrayspointers
user2954467
user2954467

Reputation: 95

c++ Dynamic array using strings won't accept strings

I am trying to put the strings in a temporary array into a dynamic array. But the compiler just breaks when it hits that.

where dynaicArray is called:

string* dynamicArray = NULL;

Here is where it is breaking:

for (int i = 1; i <= (size); i++)
{
dynamicArray[i] = tempArray[i];
}

Where tempArray is filled:

void populateArray(int& size, string*& dynamicArray)
{
char decide;
string tempArray[100]; //Holds the strings until the size is known
bool moreStrings = true;
while (moreStrings == true)
{
    cout << "\nEnter your string here:";
    cin >> tempArray[size];
    cout << "\nDo you want to enter another string? Y/N:";
    cin >> decide;
    decide = toupper(decide);
    size ++;
    dynamicArray = new string[size];
    if (decide == 'N')
    {
        for (int i = 1; i <= (size); i++) //moves all of the strings from tempArray to dynamicArray
        {
            string temp;
            temp = tempArray[i];
            dynamicArray[i] = temp;
        }
        moreStrings = false;
    }
}
}

PS: I know vectors are better. Unfortunately they're not an option.

Upvotes: 1

Views: 121

Answers (1)

Carl Colijn
Carl Colijn

Reputation: 1607

Some design ideas:

  • the code in the if (decide == 'N') block is better placed after the while, to make the while smaller == more readable
  • once the above is implemented, you can set the moreStrings var directly with the result of your decide == 'N'; no need for an explicit if there anymore
  • you now do a dynamicArray = new string[size]; in each pass through the while, which is an enormous memory leak (you'r overwriting the newly created dynamic array with a new copy without reclaiming the old one out first - see dalete[])
  • as already mentioned: don't assume 100 will be enough - read "Buffer overflow" (only viable solution: make it a dynamic array as well and re-allocate it to a bigger one if it gets full)
  • better initialize size in the function before you use it - much safer; callers don't need to remember to do it themselves
  • C++ arrays are 0-based, so when you start copying them you'd also better start at 0 and not at 1
  • nitpick: for (int i = 1; i <= (size); i++): the () around size are superfluous
  • bonus advanced nitpick: use ++size and ++i in these contexts; it's a bit more efficient
  • you now use the var tmp to copy from the temp array to the dynamic one and the code is also somewhat structured to suggest you're using it to swap the strings between the two arrays (you're not) - rename the tmp variable or get rid of it altogether

Upvotes: 1

Related Questions