1

I have a program that creates a list of 25 random numbers from 0 to 1,000. I have to buttons the first button will load a list box with the random numbers and the second button will sort the list of numbers from the smallest to largest which is where I implemented bubble sort code. Now the other list box that is supposed to hold the sorted numbers doesn't work properly it only shows one number instead of all of them.

Here is my code:

Option Strict On
Public Class Form1

Dim rn As Random = New Random
Dim Clicked As Long = 0
Dim numbers, sort As Long


Private Sub GenerateBtn_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles GenerateBtn.Click
    Clicked += 1

    For x = 0 To 25
        numbers = rn.Next(0, 1000)
        RandomBox.Items.Add(numbers)
        If Clicked >= 2 Then
            RandomBox.Items.Clear()
            Clicked = 1
        End If
    Next
End Sub


Private Sub SortBtn_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles SortBtn.Click
    Dim Sorted() As Long = {numbers}
    Dim Swapped As Boolean
    Dim endOfArray As Integer = Sorted.Length - 1
    Dim Tmp As Byte

    While (Swapped)
        Swapped = False
        For I = 0 To endOfArray - 1
            If Sorted(I) > Sorted(I + 1) Then
                Tmp = CByte(Sorted(I))
                Sorted(I) = Sorted(I + 1)
                Sorted(I + 1) = Tmp
                Swapped = True
            End If
            endOfArray = endOfArray - 1
        Next
    End While

    SortBox.Items.Clear()

    For I = 0 To Sorted.Count - 1
        SortBox.Items.Add(Sorted(I))
    Next

End Sub
End Class
Jay
  • 109
  • 1
  • 10
  • The only explanation is that `Sorted` is `null`. Are you *certain* that `GenerateBtn_Click` has been executed (and `Sorted` instantiated) prior to `SortBtn_Click`? – Martin Jul 09 '15 at 14:50
  • 1
    @MartinParkin `Sorted` is instantiated, but it's a different `Sorted`. – GSerg Jul 09 '15 at 14:51
  • Acutally, looking at your code, you declare `Sorted()` as a new variable inside `GenerateBtn_Click`... Therefore, your public declaration of `Sorted` is never made – Martin Jul 09 '15 at 14:51
  • @GSerg was just typing that as you commented :) – Martin Jul 09 '15 at 14:51
  • 4
    This line _Dim Sorted() As Long = {numbers}_ creates an array of long with just one element, and at every loop you reinitialize it. Also if you fix the local variable problem, your sorting will sort one element. I suggest to use a List(Of Long) and the Linq OrderBy extension – Steve Jul 09 '15 at 14:54
  • I filled up the array by doing this numbers = rn.Next(0, 1000) RandomBox.Items.Add(numbers) Dim Sorted() As Long = {numbers} I don't know if i am correct because I am new to Visual Basic – Jay Jul 09 '15 at 15:02
  • I can't use lists yet because we havent learned to use them and my professor would give us a 0 – Jay Jul 09 '15 at 15:03
  • `Dim` declares a new variable. All those `Dim Sorted` statements inside procedures are creating new variables which live only in that procedure. The one in the loop creates it over and over. See [Scope in Visual Basic](https://msdn.microsoft.com/en-us/library/1t0wsc67.aspx) – Ňɏssa Pøngjǣrdenlarp Jul 09 '15 at 15:31

2 Answers2

1

Change your:

Dim Sorted() As Long = {numbers}

to

Sorted(x) = numbers

edit: Since you changed your code. You need to put back in the line that loads the Sorted Array.

For x = 0 To 25
    numbers = rn.Next(0, 1000)
    RandomBox.Items.Add(numbers)
    Sorted(x) = numbers
    If Clicked >= 2 Then
        RandomBox.Items.Clear()
        Clicked = 1
    End If
Next

and remove the:

Dim Sorted() As Long = {numbers}

from the second part and put this declaration back in the beginning like you had:

Dim Sorted(26) as Long

The way you have will only show the latest random number. It is not any array but a single entity. Therefore only the latest will be add into the array. You need to load each number into the array as you create each one. Thus the (x) which loads it into position x.

Scott Craner
  • 148,073
  • 10
  • 49
  • 81
0

You didn't use any arrays at all in your project...you're using the ListBox as your storage medium and that's a really bad practice.

I recommend you set it up like this instead:

Public Class Form1

    Private rn As New Random
    Private numbers(24) As Integer ' 0 to 24 = 25 length

    Private Sub GenerateBtn_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles GenerateBtn.Click
        For x As Integer = 0 To numbers.Length - 1
            numbers(x) = rn.Next(0, 1000)
        Next

        ' reset the listbox datasource to view the random numbers
        RandomBox.DataSource = Nothing
        RandomBox.DataSource = numbers

        ' empty out the sorted listbox
        SortBox.DataSource = Nothing
    End Sub

    Private Sub SortBtn_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles SortBtn.Click
        ' make a COPY of the original array that we will sort:
        Dim sorted(numbers.Length - 1) As Integer
        Array.Copy(numbers, sorted, numbers.Length)

        Dim Swapped As Boolean = True
        Dim endOfArray As Integer = Sorted.Length - 1
        Dim Tmp As Integer

        While (Swapped)
            Swapped = False
            For I As Integer = 0 To endOfArray - 1
                If sorted(I) > sorted(I + 1) Then
                    Tmp = sorted(I)
                    sorted(I) = sorted(I + 1)
                    sorted(I + 1) = Tmp
                    Swapped = True
                End If
            Next
            endOfArray = endOfArray - 1
        End While

        ' reset the listbox datasource to view the sorted numbers
        SortBox.DataSource = Nothing
        SortBox.DataSource = sorted
    End Sub

End Class

Also, note that you were decrementing endOfArray inside your for loop. You should only decrement it after each pass; so outside the for loop, but inside the while loop.

Additionally, you were using a Tmp variable of type Byte, but generating numbers between 0 and 999 (inclusive). The Byte type can only hold values between 0 and 255 (inclusive).

Your Bubble Sort implementation was very close to correct!

Idle_Mind
  • 38,363
  • 3
  • 29
  • 40