My program throws an ArrayIndexOutOfBoundsException, but only sometimes

Question

I'm taking a computer science class at my high school, and we learned a lesson on selection sort today. I wrote a program (it's probably clumsy, but please bear with me, I'm learning) and it works in that it sorts, but sometimes it throws an ArrayIndexOutOfBoundsException. Only sometimes. I don't know how this is possible, because I deal with the same array throughout the entire program and arrays have a fixed length. If anyone has some insight it would be extremely helpful.

I think the error has something to do with int first = y[movedVariable];. However, I don't understand how movedVariable can be out of bounds, because I'm pretty sure I wrote my program so that it would be < the length of the array.

public class selectionSort
{
    public static int[] array;
    public static int movedVariable = 0;
    public static void main()
    {
        array = new int[10];
        int x;
        for (int count = 0; count < array.length; count++)
        {
            if (count == 0)
            {
                x = (int)(Math.random()*100+2);
                array[count] = x;
            }
            else
            {
                x = (int)(Math.random()*100+2);
                for (int y = 0; y < count; y++)
                {
                    while(x == array[y])
                    {
                        x = (int)(Math.random()*100+2);
                    }
                }
                array[count] = x;
            }
        }
        sort(array);
    }

    public static void sort(int[] x)
    {
        int thing = 0;
        for(int hello = 0; hello < x.length; hello++)
        {   
            int part = x[thing];
            for ( int count = thing; count < x.length-1; count++)
            {
                if( part > x[count+1] )
                {
                    part = x[count+1];
                }
            }
            thing++;
            swap( x, part);
        }

        int f = 0;
        String output = "";

        for( int val : x )
        { 
            if (f%10 == 0)
            {output += "\n";}
            output += val + " "; 
            f++;
        }
        System.out.print(output);
    }

    public static int[] swap(int [] y, int num)
    {
        int count = 0;
        int index = 0;
        for ( count = 0; count < y.length; count++)
        {
            if (y[count] == num)
            {index = count;}
        }
        int first = y[movedVariable];
        y[movedVariable] = y[index];
        y[index] = first;
        movedVariable++;
        return y;
        }
    }

Answer 1

For fun, I ran your code for 1,000,000 iterations and no out of bounds exception, unless I did not clear the static movedVariable to 0 before each iteration.

Since movedVariable is static after the first 10 calls to swap() it will be 10 and if another call is made to swap you'll get the index out of bounds. However, this can only happen if you call sort() more than once per run. Only use static for values that need to be preserved between instances of your class. Non statics that are part of the your instance state. Local variables for everything else. Otherwise your are creating a mine field of bugs just waiting to happen.

I refactored your class to remove variables that have the same functionality. For example your thing and your movedVariable and your hello variable in sort() can be just one variable. Try to eliminate multiple variables that do the same thing, like the plague. It is a source of non obvious bugs.

Also, you are passing the value in the array to swap then looking for it in the array to get the index, this is a waste of time. Just pass in the index to swap. It also creates a problem for your sort function when you have the same value at two different places. Swap will use the last one it finds. sort() should handle duplicate values in the array. That explains why you initialized your array with unique values. You should not have to do that. Actually you should test your code with duplicates explicitly added to make sure your function works.

I moved printing of the array out of sort into its own method. It is useful for debugging at intermediate steps not just when the sort is done.

I tried to leave variable names the same and the logic unchanged so you can follow the changes.

public class Main
{
    public static void sort(int[] x)
    {
        for (int movedVariable = 0; movedVariable < x.length; movedVariable++)
        {
            int part = x[movedVariable];
            int index = movedVariable;

            for (int count = movedVariable; count < x.length - 1; count++)
            {
                if (part > x[count + 1])
                {
                    part = x[count + 1];
                    index = count + 1;
                }
            }

            swap(x, index, movedVariable);
        }

        printArray(x);
    }

    private static void printArray(int[] x)
    {
        int f = 0;
        String output = "";

        for (int val : x)
        {
            if (f % 10 == 0)
            {
                output += "\n";
            }
            output += val + " ";
            f++;
        }
        System.out.print(output);
    }

    public static int[] swap(int[] y, int index, int movedVariable)
    {
        int first = y[movedVariable];
        y[movedVariable] = y[index];
        y[index] = first;
        return y;
    }

    public static void main(String[] args)
    {
        int[] array = new int[10];
        int x = 0;
        for (int count = 0; count < array.length; count++)
        {
            for (int y = count; --y >= 0; )
            {
                do
                {
                    x = (int) (Math.random() * 100 + 2);
                }
                while (x == array[y]);
            }

            array[count] = x;
        }

        printArray(array);
        sort(array);
    }
}