Problem with bubble sort code

[flyingcurry]

The method below for bubble sort should work, it displays the unsorted array fine, but for the sorted array it only displays one of the numbers. It seems to have just skipped over the other elements in the array.

Here is a sample output:
How many random even integers would you like to generate? 6
Unsorted:
196
166
68
14
68
176
Sorted:
196
5

public static void bubbleSort (int [] array, int length) {

		int temp, i, j;

		System.out.println("Unsorted: ");
		for (i=0; i<array.length; i++)
		{
			System.out.println(array[i]);

		}

		for (i=0; i<length-1; i++){
			for (j=0; j<length-1; j++)
				if (array[j]>array[j+1]){
					temp=array[j];
					array[j]=array[j+1];
					array[j+1]=temp;
				}
		}
		System.out.println("Sorted: ");
		for (i=0; i<length-1; i++);
		{
			System.out.println(array[i]);
			System.out.println(i);
		}
	}

[kramerd]

You have a misplaced semicolon at the end of line 21. That means the loop body does nothing, and then after the loop executes, you print the last element of the array and the value of i (which is 5 after the loop executes). I'm guessing that's why your loops are stopping at length-1, because you were getting an ArrayIndexOutOfBounds exception on line 23?

Remove the semicolon at the end of line 21, and change the stopping condition of the loop to i < length; .

[apines]

kramerd is right - your semicolon at the end of the for loop at line 21 makes the loop to do nothing. Since you have declared i and j as members outside of the loop instead of creating them for each loop, the value of i at the end of the loop is the index of your last cell.
Few comments about the code if I may: First, you don't have to declare i and j the way you did, you can declare them in each loop separately:

public static void bubbleSort (int [] array, int length) 
{
   int temp; //here we are not declaring i and j.
   System.out.println("Unsorted: ");
   for (int i=0; i<array.length; i++) //notice the int i instead of just i.
   {
      System.out.println(array[i]);
   }
   for (int i=0; i<length-1; i++) //here as well we have int. Every loop we are redeclaring i in the loop's scope.
   {
      for (int j=0; j<length-1; j++)
      {
	 if (array[j]>array[j+1])
         {
	    temp=array[j];
	    array[j]=array[j+1];
	    array[j+1]=temp;
         }
      }
   }
   System.out.println("Sorted: ");
   for (int i=0; i<length-1; i++);
   {
      System.out.println(array[i]);
      System.out.println(i);
   }
}

If you would have done so this way, you would have gotten a compiler error since i would not have existed in that scope.
Second remark is that in Java you don't have to send the length of an array as a parameter, you can simply use array.length. Therefore, since the parameter you send is always the length of the array, you can change your code to accept only one parameter:

public static void bubbleSort (int[] array) 
{
   int temp;
   int length = array.length; //our length variable, computed inside the method instead of being sent as a parameter.

   //rest of the code is unchanged
   System.out.println("Unsorted: ");
   for (int i=0; i<array.length; i++)
   {
      System.out.println(array[i]);
   }
   for (int i=0; i<length-1; i++) 
   {
      for (int j=0; j<length-1; j++)
      {
	 if (array[j]>array[j+1])
         {
	    temp=array[j];
	    array[j]=array[j+1];
	    array[j+1]=temp;
         }
      }
   }
   System.out.println("Sorted: ");
   for (int i=0; i<length-1; i++);
   {
      System.out.println(array[i]);
      System.out.println(i);
   }
}

[tong1]

(1) flyingcurry ,if you have to sort the whole array the second argument “length” is redundant. In fact, the length is the array.length。
(2) The line 13 could be replaced with

for (j=0; j<length-1-i; j++)  //to avoid the unnecessary comparisons made of the stable region.

See the post on bubble sort

The line 21 should be replaced with

for (i=0; i<length; i++)

[flyingcurry]

Since you have declared i and j as members outside of the loop instead of creating them for each loop, the value of i at the end of the loop is the index of your last cell.
The line 13 could be replaced with
for (j=0; j<length-1-i; j++) //to avoid the unnecessary comparisons made of the stable region.

Thanks, i realize too that making a variable for array.length was unnecessary now.
The semi-colon was an bad typing mistake.

Apines, I am still not understanding why declaring i and j at the start of the method does not work though, "the value of i at the end of the loop is the index of your last cell." What do you mean by index of last cell?

Tong, can you explain what is meant by "unnecessary comparisons made of the stable region"? I saw some other bubble sort codes online doing this, but I didn't really understand why?

Thanks very much!

[apines]

I didn't say that it doesn't work, I said it's bad coding. When you declare i and j outside the loop, they are accessible everywhere in the method, while declaring them in the loop makes them accessible only inside the loop.
In your case, where you put the extra semicolon, the line simply incremented i from 0 to length-1, and then outside of the loop (again, because of the semicolon the next line was considered outside of the loop) you have printed the last cell of the array. On the other hand, if i and j were declared only in the loop initialization, your code would have returned a compilation error, since i and j would not have existed outside the loop, and you would have attempted to access array - where i does not exist.