Do While Loop doesn't stop looping

[JL.19]

I'm trying to generate a random number that's not in the database. If the randomly generated number happens to already be in the database, a message box appears saying the number exists. When you click Ok, it generates another number and if it's still in the database, it will repeat the same process. With my code, it keeps showing the message box and generating a number even after it has already generated a number that's not in the database. This is my code:

Private Sub BtnOrder_Click(sender As Object, e As EventArgs) Handles BtnOrder.Click
        Dim rand As New Random
        Dim num As Integer

        num = rand.Next(1, 30)
        TxtOrder.Text = "#" + num.ToString("0000")
        BtnOrder.Enabled = False

        Dim connection As New OleDbConnection("Provider=Microsoft.ACE.OLEDB.12.0;Data Source=Daily Sales.accdb;")
        Dim command As New OleDbCommand("SELECT [Order No] FROM [Table1] WHERE [Order No] = orderno", connection)
        Dim orderParam As New OleDbParameter("orderno", Me.TxtOrder.Text)
        command.Parameters.Add(orderParam)
        command.Connection.Open()
        Dim reader As OleDbDataReader = command.ExecuteReader()
        Do While reader.HasRows = True
            If reader.HasRows = False Then
                Exit Do
            End If
            MessageBox.Show("Order number exists.", "Invalid Order Number")
            num = rand.Next(1, 30)
            TxtOrder.Text = "#" + num.ToString("0000")
        Loop
        command.Connection.Close()
    End Sub

[dday9]

I would not suggest doing this. What I would suggest doing is:

1. Get every Order No from your database.
2. Massage the values to remove the pound sign and to convert the string values to integer.
3. Then create a new range of numbers and filter out the numbers from step 2.
4. Finally, randomly order your filtered collection from step 3 and grab the first value

This away you only make one database call and don't have to use a Do loop.

Take a look at this example:

Dim orderNumbers = New List(Of Integer)
Try
    Using connection = New OleDbConnection("Provider=Microsoft.ACE.OLEDB.12.0;Data Source=Daily Sales.accdb;")
        Using command = New OleDbCommand("SELECT mid([Order No], 1, 64) FROM [Table1]", connection)
            connection.Open()

            Dim reader As OleDbDataReader = command.ExecuteReader()
            While reader.Read()
                Dim orderNumber = reader(0).ToString()
                Dim orderNumberConverted As Integer
                If (Integer.TryParse(orderNumber, orderNumberConverted)) Then
                    orderNumbers.Add(orderNumberConverted)
                End If
            End While

            connection.Close()
        End Using
    End Using
Catch ex As Exception
    ' do something with exception
End Try

Dim uniqueNumbers = Enumerable.Range(1, 30).Where(Function(i) Not orderNumbers.Contains(i)).OrderBy(Function(i) rand.Next())
Dim uniqueNumber = uniqueNumbers.FirstOrDefault()

[Shaggy Hiker]

I agree.

The problem with the Do loop, as you have it, is that you generate a new number, but you don't then check it against the database. You'd have to move the Do statement higher such that you are re-querying with the new number. But even doing that would be a really bad idea. As long as there are few records, the performance would be pretty good. Once you get a dozen, or more, records, the performance of that loop is going to get noticeably worse and worse until it takes an inordinately (and unpredictably) long time.

[techgnome]

Well, the problem with that loop is deeper than the others go into:

        Do While reader.HasRows = True
            If reader.HasRows = False Then
                Exit Do
            End If
            MessageBox.Show("Order number exists.", "Invalid Order Number")
            num = rand.Next(1, 30)
            TxtOrder.Text = "#" + num.ToString("0000")
        Loop

.HasRows() never changes ... ever. It's set when the .Execute happens and indicates if there are rows on the reader or not. You then never .Read() from it, so it never advances. HasRows should be used in an If statement... not a loop.... Read() is what should have been used instead. But as the others mentioned, even once you generate a new number, you don't re-check it against the database anyways, so it's largely useless in what it does. Especially since you only use a range of 1-29.

-tg

[Shaggy Hiker]

I didn't go into the details of why the Do loop has to be moved, but that is the reason. Looping on HasRows wouldn't be, in itself, horrible as long as the DB is re-queried all the times. It would be kind of useless, but it wouldn't be horrible. Still, since Read was never called, it was good that no other interaction with the datareader happened, as it would have crashed if any part of the current row was accessed, as there wouldn't be a current row.

I've never seen much point to HasRows. The situation where that is in any way better than Read seems vanishingly rare. Sure, Read my cost a bit more, since it actually GETS a row, rather than just saying whether there is one, but how often do you create a datareader and NOT want at least the first row?

[ChrisE]

IMO you have to use a Select Count

here a sample
->check if Studentname is in Database Table
--> if not add
and give the User some notification(added or not) along this process. You will have to adapt this to your need's

 Private Sub Button1_Click(sender As System.Object, e As System.EventArgs) Handles Button1.Click
        Dim sDB As String = "E:\Adressen.mdb"
        Dim sCon As String = "Provider=Microsoft.Jet.OLEDB.4.0;" & _
                                       "Data Source=" & sDB & ";"
        Dim Cn As OleDb.OleDbConnection = New OleDb.OleDbConnection(sCon)
        Cn.Open()

        'the Sql for checking in tha Database table
        Dim sSql As String = "select count(*) from tbl_Student" & _
         " Where StudentName = @StudentName"

        'is anything in Textbox to check ?
        If TextBox1.Text = Nothing Then
            MessageBox.Show("no Value to check !", "Error", _
                            MessageBoxButtons.OK, _
                            MessageBoxIcon.Error)
            'nothing there so Exit
            Exit Sub
        End If

        Using cmd As New OleDbCommand(sSql, Cn)
            cmd.Parameters.AddWithValue("@StudentName", TextBox1.Text)

            Dim Result As String = CStr(cmd.ExecuteScalar)

            If CDbl(Result) = 0 Then
                'not found in Database, so start Insert
                Using cmd1 As OleDbCommand = New OleDbCommand("INSERT INTO tbl_Student (StudentName) VALUES (@StudentName)", Cn)
                    cmd1.Parameters.AddWithValue("@StudentName", TextBox1.Text)
                    cmd1.ExecuteNonQuery()
                    'Close the connection
                    'just show you Inserted the Record
                    MessageBox.Show("User :" & TextBox1.Text & " added", "Add User..", _
                                    MessageBoxButtons.OK, _
                                    MessageBoxIcon.Information)
                    Cn.Close()
                End Using
            Else
                'show User is already in Database Table
                MessageBox.Show("Username -> " & TextBox1.Text & " <-  is already in Database", "Error", _
                                            MessageBoxButtons.OK, _
                                            MessageBoxIcon.Exclamation)
            End If
        End Using
        Cn.Close()
    End Sub