Array causing program to crash

Currently working my way through the Jumping into C++ e-book, anyways the current chapter is on arrays, in said chapter we are given a way of sorting the variables in our array and then returning them in order from smallest to largest. The practice question asks us: Turn the code that we wrote for insertionSort into an insertionSort function that works for any sized array.

Now I thought this would be easy, but alas I was wrong, I cannot seem to figure out where to go with this one. The code below is what I have gotten so far, but my program is crashing so clearly it is not right... I thought that changing the for loop in int main from a hard coded i < 10 to a value that was input by the user would solve this practice problem, but I was wrong. Original: for ( int i = 0; i < 10; i++ ) ///// Mine: for ( int i = 0; i < arrayLength; i++ )

What am I not understanding here??? (code below)

Code:

#include <cstdlib>
#include <ctime>
#include <iostream>
using namespace std;
int findSmallestRemainingElement (int array[], int size, int index);
void swap (int array[], int first_index, int second_index);
void sort (int array[], int size)
{
    for ( int i = 0; i < size; i++ )
    {
        int index = findSmallestRemainingElement( array, size, i );
        swap( array, i, index );
    }
}
int findSmallestRemainingElement (int array[], int size, int index)
{
    int index_of_smallest_value = index;
    for (int i = index + 1; i < size; i++)
    {
        if ( array[ i ] < array[ index_of_smallest_value ] )
        {
            index_of_smallest_value = i;
        }
    }
    return index_of_smallest_value;
}
void swap (int array[], int first_index, int second_index)
{
    int temp = array[ first_index ];
    array[ first_index ] = array[ second_index ];
    array[ second_index ] = temp;
}
// small helper method to display the before and after arrays
void displayArray (int array[], int size)
{
    cout << "{";
    for ( int i = 0; i < size; i++ )
    {
    // you'll see this pattern a lot for nicely formatting
    // lists--check if we're past the first element, and
    // if so, append a comma
    if ( i != 0 )
    {
        cout << ", ";
    }
    cout << array[ i ];
    }
    cout << "}";
}
int main ()
{
    int arrayLength;
    int array[ arrayLength ];
    cout << "Please tell me how many numbers you would like in your array today: ";
    cin >> arrayLength;
    cout << "Thank you! \n";
    srand( time( NULL ) );
    for ( int i = 0; i < arrayLength; i++ )
    {
        // keep the numbers small so they're easy to read
        array[ i ] = rand() % 100;
    }
    cout << "Original array: ";
    displayArray( array, arrayLength );
    cout << '\n';
    sort( array, arrayLength );
    cout << "Sorted array: ";
    displayArray( array, arrayLength );
    cout << '\n';
}

Originally Posted by rTeapot

Code:

int main ()
{
    int arrayLength;
    int array[ arrayLength ];
    cout << "Please tell me how many numbers you would like in your array today: ";
    cin >> arrayLength;
    cout << "Thank you! \n";
    srand( time( NULL ) );
    for ( int i = 0; i < arrayLength; i++ )
    {
        // keep the numbers small so they're easy to read
        array[ i ] = rand() % 100;
    }

Arrays don't work like that in C++. The size of an array must be known at compile time. You cannot change the size of an array at runtime like you're attempting to do.
If you need dynamically resizable "arrays" either use std::vector (recommended) or operator new [] and delete [] (_NOT_ recommended!).


P.S. I'm surprised you even got that to compile! I suggest you throw your compiler out the window and get a proper C++ compiler.

Originally Posted by antred

P.S. I'm surprised you even got that to compile! I suggest you throw your compiler out the window and get a proper C++ compiler.

Perhaps another one of those students being forced to use Turbo C++ |3

Thank you, I will try attempting this some other ways.

Not just some other ways. Use a vector:

Code:

#include <iostream>
#include <vector>
#include <cstdlib>
#include <ctime>
#include <stdexcept>

int main()
{
    try
    {
        std::vector< int > myArray;
        std::size_t arrayLength = 0;

        std::cout << "Please tell me how many numbers you would like in your array today: ";
        std::cin >> arrayLength;
        std::cout << "Thank you!\n";

        std::srand( std::time( NULL ) );

        // optional optimization ... make sure vector has a sufficiently large capacity to avoid reallocations while pushing data onto the vector
        myArray.reserve( arrayLength );

        for ( std::size_t i = 0; i < arrayLength; ++i )
        {
            // keep the numbers small so they're easy to read
            myArray.push_back( std::rand() % 100 );
        }
    }
    catch ( const std::exception& ex )
    {
        std::cerr << "std::exception caught! " << ex.what() << std::endl;
        return 1;
    }
    catch ( ... )
    {
        std::cerr << "Unspecified exception caught!\n";
        return 1;
    }
}

EDIT: Added link to documentation of the interface of std::vector: http://www.cplusplus.com/reference/stl/vector/

Compiler extensions not withstanding, it would be better if you INPUT a length BEFORE trying to create an array of that length.

Code:

    int arrayLength;
    int array[ arrayLength ];  //!! what length?
    cout << "Please tell me how many numbers you would like in your array today: ";
    cin >> arrayLength;

vs.

Code:

    int arrayLength;
    cout << "Please tell me how many numbers you would like in your array today: ";
    cin >> arrayLength;
    int array[ arrayLength ];

Portably, you would do this

Code:

    int arrayLength;
    cout << "Please tell me how many numbers you would like in your array today: ";
    cin >> arrayLength;
    int *array = new int[arrayLength];

Or better, use a std::vector as shown by andred.

Originally Posted by antred

Not just some other ways. Use a vector:
EDIT: Added link to documentation of the interface of std::vector: vector - C++ Reference

You .resize() instead of .reserver(). .reserve() allocates memory for n elements, but does not create them.
Therefore, accessing any element [0, n) results in undefined behavior if you use .reserve().
Also, I strongly recommend the usage of std::vector for dynamic arrays and std::array (works if you have a somewhat recent compiler) for normal arrays. They usually aid in debugging when you get out-of-bounds access.

Originally Posted by Elysia

You .resize() instead of .reserver(). .reserve() allocates memory for n elements, but does not create them.
Therefore, accessing any element [0, n) results in undefined behavior if you use .reserve().

Yes, but note that in my example I push_back() after I reserve().

Then your reserve is just an optimization (or not, depending on circumstances).

Yep, which is exactly what it says in the comment above the reserve() call.

My bad for not reading the code properly >_<

No problem.