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

Question

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.

Whenever I hear the words "dynamic array" in C++ I always think std::vector. Maybe you should too? — Some programmer dude
– Some programmer dude, Commented Feb 13, 2014 at 8:45
what do you mean by "the compiler breaks"? please show the error — WeaselFox
– WeaselFox, Commented Feb 13, 2014 at 8:46
If you'd use a std::vector<string> in stead of a dynamic array, you wouldn't need the temporary array either, making your code much more robust (what if the user enters more than 100 strings?) — Carl Colijn
– Carl Colijn, Commented Feb 13, 2014 at 8:47
string tempArray[100]; and while (moreStrings == true) - hmm... — bobah
– bobah, Commented Feb 13, 2014 at 8:47

Carl Colijn · Accepted Answer · 2014-02-13 09:25:19Z

1

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

edited Feb 13, 2014 at 9:25

answered Feb 13, 2014 at 9:14

Carl Colijn

1,67212 silver badges34 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Tadeusz Kopec for Ukraine Over a year ago

Nitpick to nitpick for (int i = 1; i <= (size); i++) has much bigger problem than superfluous parenthesis - it should be for (int i = 0; i < size; i++)

Carl Colijn Over a year ago

Yeah, I already addressed that, though it's buried somewhat in there between the rest ;)

Collectives™ on Stack Overflow

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

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related