2

I am learning C++ coming over from Objective-C / C, and for a dummy project I want to load the words from the /usr/share/dict/words file stored on Mac OS X machines.

The idea is to load the file and get each word into an array, so I have an array of type string.

But I'm having trouble working correctly with dynamic memory with my arrays - using new and delete. I've added some of the code below, if anyone could help out...

And so I'm getting a memory error:

word:: A
word:: a
word:: aa
word:: aal
definitions(2758) malloc: *** error for object 0x100103b90: incorrect 
checksum for freed object - object was 
probably modified after being freed.
*** set a breakpoint in malloc_error_break to debug

Load words:

string* Definition::loadWords()
{
    int arrayLength = 0;

    arrayOfWords = new string[arrayLength];

    ifstream file;

    file.open("/usr/share/dict/words");

    if(file.is_open())
    {
        while(file.good()){
            string word;
            getline( file, word );
            this->addWord(word, arrayOfWords, &arrayLength);
        }

    }

    file.close();

    cout << endl << "There are " << arrayLength << " words" << endl;

    return arrayOfWords;
};

Add words to array:

void Definition::addWord(string newWord, string currentArray[], int* arrayLength)
{
    cout << endl << "word:: " << newWord;

    string *placeholderArray = new string[*arrayLength + 1];
    placeholderArray[*arrayLength + 1] = newWord;

    for(int i = 0; i < *arrayLength; i++){
        placeholderArray[i] = currentArray[i];
    }

    (*arrayLength)++;

    currentArray = placeholderArray;

    delete [] placeholderArray;
}
Improve this question
1
  • Switch to using vectors instead of using dynamic memory directly. Commented Feb 19, 2013 at 2:44

3 Answers 3

Reset to default
3

First thing I can see is this:

placeholderArray[*arrayLength + 1] = newWord;

You are adding an element past the end of the array. Arrays are indexed from 0. For example if the array length is 5 then the last element in the array is at index 4. So the line should be:

placeholderArray[*arrayLength] = newWord;

Then after that you are deleting your array with this:

currentArray = placeholderArray;

delete [] placeholderArray;

Since you are just setting currentArray to point to placeholderArray and then deleting it.

Also passing by reference is much better than passing by pointer. So rather than this:

void Definition::addWord(string newWord, string currentArray[], int* arrayLength)

Use this:

void Definition::addWord(string newWord, string currentArray[], int& arrayLength)

That you don't always have to use the * to get the value each time you want to use it.

Here's a tutorial on using references: http://www.learncpp.com/cpp-tutorial/73-passing-arguments-by-reference/

Also save yourself the time and effort and learn to use vectors and stl containers rather than arrays sooner rather than later.

Here's a tutorial for using vectors: http://www.codeguru.com/cpp/cpp/cpp_mfc/stl/article.php/c4027/C-Tutorial-A-Beginners-Guide-to-stdvector-Part-1.htm

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

3 Comments

Hi, thanks, you're incorrect about your first observation, I am using arrayLength + 1 which is from the previous array, I've already created a new array with +1 elements, the idea was to put the new word last before I copy the previous arrays words from 0 to n - 1. Thank you for pointing out about reference vs pointers, I missed that.
Try step through your program and you will see what I'm saying. arrayLength starts at 0, you then add a string which passes 0 as arrayLength. You the create an array of length arrayLength+1 (1) and then you add an element at arrayLength+1 (1), which means you have just added an element beyond the end of the array. So you should be creating the array of length 1 and adding element at index 0.
Actually, David's correct there. For instance, the first time the method is called, you would create array[1]. Then you set a value to array[1], which is out of bounds.
1

Here you are simply assigning the pointer, as opposed to the values in the array:

currentArray = placeholderArray;

And here you free the space pointed to by said pointer:

delete [] placeholderArray;

The next time you to read from the freed memory space will result in undefined behavior.

Instead of using C-style arrays in C++, use std::vector and its resize() function. Better still, your application could simply invoke push_back() on each newWord, which will obviate the entire need for your addWord() function.

Improve this answer

Comments

1 
currentArray = placeholderArray;

This aliases placeholderArray to currentArray. So, when you call...

delete [] placeholderArray;

.. you are deleting what currentArray is pointing to.

Improve this answer

4 Comments

So what I should be doing is not deleting it, but then I still have a crash, I run out of memory
Does your assignment require you to use dynamic arrays in this way? If you can use vectors, that would be easier.
There's no assignment, actually I'm just trying to master dynamic memory, which is why this vector solution isn't interesting for me right now.
In that case, return the second array and delete the original.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.