[RESOLVED] Help with datareader

[crater]

I need a little help with this.
I don't usually use a DataReader in this way but now have a need for a one way, forward communication this allows.

Anyway I got this code for the reader off the net but what it doesn't seem to handle is the null values in the db.
The repro is:
I do have a matching username and pcname but the user choose not to password protect, so the Password field is null.

Private Sub btnLogin_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnLogin.Click

        If txtUsername.Text = "" Then
            MessageBox.Show("Please complete the required fields..", "Authentication Error", MessageBoxButtons.OK, MessageBoxIcon.Error)
        Else

            ' Check if user exist in OneData database
            ' Connect to DB

            Dim conn As New System.Data.OleDb.OleDbConnection()
            conn.ConnectionString = My.Settings.OneDataConnectionString


            Try
                'Select the Staff record that matches the username, pcname and password
                Dim sql As String = "SELECT * FROM IDStaff WHERE StaffName= '" & Trim(txtUsername.Text) & "'" & _
                    " AND PCNAME= '" & Trim(txtPcName.Text) & "'" & " AND [Password] = '" & Trim(txtPassword.Text) & "'"

                Dim sqlCom As New System.Data.OleDb.OleDbCommand(sql)

                'Open Database Connection
                sqlCom.Connection = conn
                conn.Open()

                Dim sqlRead As System.Data.OleDb.OleDbDataReader = sqlCom.ExecuteReader()
              
                'Check if this user is in the database
                If sqlRead.Read() Then

                    'Yes, username/password/pcname check has passed
                    UserModule.PCName = txtPcName.Text
                    UserModule.UserName = txtUsername.Text

                    Me.Close()

                Else
                    ' No username/password/pcname checked failed
                    ' Throw an error message
                    MessageBox.Show("The Username/MachineName combination is either incorrect or not registered in the system..", "Authentication Failure", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)

                    'Focus on Username field
                    txtUsername.Focus()

                End If

                ' Close and dispose of the connection resource
                conn.Close()
                conn.Dispose()

            Catch ex As Exception
                MessageBox.Show("Failed to connect to Database..", "Database Connection Error", MessageBoxButtons.OK, MessageBoxIcon.Error)
            End Try

        End If

    End Sub

any guidance in using a datareader and dbnulls?
I know how to check a data row for dbnull but since I'm not going that route leaves me at a lose.

[dday9]

You don't need a DataReader for this, take a look at this example:

Private Function RowCount(ByVal username As String, ByVal pcName As String) As Integer
    'Declare the object to return
    Dim count As Integer = -1

    'Declare the connection object
    Dim con As OleDbConnection

    'Wrap code in Try/Catch
    Try
        'Set the connection object to a new instance
        'TODO: Change "My Connection String Here" with a valid connection string
        con = New OleDbConnection(My.Settings.OneDataConnectionString)

        'Create a new instance of the command object
        'TODO: Change [ID] to a valid column and [MyTable] to a valid table
        Using cmd As OleDbCommand = New OleDbCommand("SELECT Count([ID]) FROM [IDStaff] WHERE [StaffName]=@staff AND [PCNAME]=@pc", con)
            'Parameterize the query
            With cmd.Parameters
                .AddWithValue("@staff", username)
                .AddWithValue("@pc", pcName)
            End With

            'Open the connection
            con.Open()

            'Use ExecuteScalar to return a single value
            count = Convert.ToInt32(cmd.ExecuteScalar())

            'Close the connection
            con.Close()
        End Using
    Catch ex As Exception
        'Display the error
        Console.WriteLine(ex.Message)
    Finally
        'Check if the connection object was initialized
        If con IsNot Nothing Then
            If con.State = ConnectionState.Open Then
                'Close the connection if it was left open(exception thrown)
                con.Close()
            End If

            'Dispose of the connection object
            con.Dispose()
        End If
    End Try

    'Return the row count
    Return count
End Function

Edit - I see that you actually need the username and PC name values. I'm going to eat and come back to this if no one's answered.

[jmcilhinney]

So, are you saying that that code is not yours and is just an example of using a data reader that you got off the web? If so then I suggest that you ignore it because it's a poor example for a number of reasons.

As for your question about null values, handling them is really quite easy. Firstly, it's important to know that null values from a database are represented in ADO.NET by an instance of the DBNull class and specifically the value returned by the DBNull.Value property. It was done this way because early versions of .NET had no way to represent no value for value types. All the data types built into VB other than String are value types, so DBNull was required to represent null values for Integer, Double, Byte, Date, etc.

That said, newer versions of .NET actually can represent no value for value types via the Nullable(Of T) class. That means that DBNull is not strictly required anymore. There's too much ADO.NET code out there for them to make a breaking change though, so it remains.

As for the issue of handling nulls, exactly what you do depends on exactly what you're trying to achieve. If a column is nullable then the first rule is that you NEVER read that column as it's actual type without testing whether it contains null first. For instance, if you have an Integer column then you could read an Integer from it like this:

Dim myInteger As Integer = myDataReader.GetInt32(columnIndex)

GetInt32 is going to throw an exception if there is no Int32 value though. That means that, for a nullable column, you could do this:

Dim myNullableInteger As Integer?
Dim fieldValue As Object = myDataReader(columnIndex)
 
If fieldValue IsNot DBNull.Value Then
    myNullableInteger = CInt(fieldValue)
End If

In this case, you get the fieldValue into an Object variable first, so that it can handle both DBNull and Integer. You then test whether it is type DBNull and, if not, cast it as type Integer and assign to the final variable. In this case, the final variable is a Nullable(Of Integer) so that it can truly have no value if the database contains null.

It's actually easier than that though, because the data reader has a test for null built into it. With the new If operator in VB.NET, it can all boil down to a one-liner:

Dim myNullableInteger = If(myDataReader.IsDBNull(columnIndex),
                           DirectCast(Nothing, Integer?),
                           myDataReader.GetInt32(columnIndex))

The IsDBNull method tells you whether the field is null or not, giving you the option to read it or not.

Whether you actually use nullable value types depends on what you're trying to achieve. If you're working with Strings for instance, a variable can actually contain Nothing because String is a reference type. Even if you're working with numbers or the like though, you might still not need to use nullable value types. If you do though, they follow a similar rule for null values, i.e. you can't get the Value property unless there is a value to get, so always test the HasValue property before using them.

[crater]

Much appreciated guys.

I took JMC's approach, modified my code and created a Class to call from my App.

can you take a look at this to see if it needs anything more.

BTY I did not design the DB, so the reason for the un-hashed password thing was not up my undertaking, I'm just working with what I was given. I personally would have hashed the password then ran it though an decryption function before testing but whatever floats there boat I guess works.

 Imports System.Data.OleDb

Public Class clsLogin

    Public LoggedIn As Boolean = False
    Public CloseApp As Boolean = False

    Public Sub LoginCheck(ByRef UserName As String, ByVal PcName As String, _
                          ByVal ConnString As String, Optional ByVal User_Supplied_Password As String = "")

        Dim Connection As OleDb.OleDbConnection = New OleDb.OleDbConnection
        Connection.ConnectionString = ConnString
        Try
            Using Connection
                Dim command As OleDb.OleDbCommand = New OleDb.OleDbCommand( _
                 "SELECT * FROM IDStaff WHERE StaffName= '" & Trim(UserName) & "'" & _
                        " AND PCNAME= '" & Trim(PcName) & "'", Connection)

                Connection.Open()

                Dim reader As OleDb.OleDbDataReader = command.ExecuteReader()

                If reader.HasRows Then
                    Do While reader.Read()

                        ' Test for Optional null Password
                        Dim myDBPassWord As String = CStr(If(reader.IsDBNull(15),
                                                       DirectCast(Nothing, String),
                                                       reader.GetString(15)))
                        If myDBPassWord <> "" Then
                            If myDBPassWord = User_Supplied_Password Then
                                LoggedIn = True
                            Else
                                Dim MsgResult1 As DialogResult = MessageBox.Show("Incorrect Password", "Authentication Error", _
                                                                   MessageBoxButtons.RetryCancel, MessageBoxIcon.Error)
                                If MsgResult1 = DialogResult.Retry Then
                                    LoggedIn = False
                                    CloseApp = False
                                ElseIf MsgResult1 = DialogResult.Cancel Then
                                    LoggedIn = False
                                    CloseApp = True
                                End If
                            End If
                        End If
                    Loop
                Else
                    Dim result As DialogResult = MessageBox.Show("User not reconized by system", "Authentication Error", _
                        MessageBoxButtons.OK, MessageBoxIcon.Error)
                    If result = DialogResult.OK Then
                        LoggedIn = False
                    End If
                End If
                reader.Close()
            End Using

        Catch ex As Exception

        End Try

    End Sub
End Class

Maybe converting this to a function would be better?

[techgnome]

1) I don't think a reader is necessary here. I would use ExecuteScalar and simply return a boolean value if the record exists or not.
2) Don't ever - EVER - tell a user that their "username is invalid" ... or that it is the "wrong password"... instead tell them that "either the username or password is incorrect." ... why? Because if I know I've got a valid username, I start cranking out the password breaker knowing the username is valid. By saying one or the other is incorrect, they don't know which is the wrong one. Keeps them guessing.
3) Yes, I'd make it a function... but I wouldn't have all of the messageboxes in there...it should simply check for the user and return true (user found and password matches) or false (one or both are mismatched). The calling code then determines what should get displayed.

-tg

[dday9]

1) I don't think a reader is necessary here. I would use ExecuteScalar and simply return a boolean value if the record exists or not.

That is initially what I suggested, but it appears as though the OP requires to store the username and PC name rather than simply confirming if a row exists with the desired username and PC name. Which now that I think about it, if the OP is entering in the username and PC name then he'd already have those two values... I think that I will re-suggest what I posted in post #2.

[crater]

My problem was in the SQL statement, where the password could be a null string, either in the db or in as user input

As dday9 suggests:

That is initially what I suggested, but it appears as though the OP requires to store the username and PC name rather than simply confirming if a row exists with the desired username and PC name

If either the UserName and PcName differs from the user input then no row is returned.
This does not account for the password field parameter.

"SELECT * FROM IDStaff WHERE StaffName= '" & Trim(txtUsername.Text) & "'" & _
" AND PCNAME= '" & Trim(txtPcName.Text) & "'" & " AND [Password] = '" & Trim(txtPassword.Text) & "'"

Was the original Select Statement, however if the password field is null then no row is returned due to the Select Statement looking for a password of "". <--Empty string in txtPassword textbox

I guessing, no I'm sure "" is not the same as null, so it seems like the isDBNull check is appropriate here to check the field for null and cast it to a type if it is. This is what I think JMC was guiding me to do anyway.

I do like the idea of just returning a Boolean value

Techgnome, after I think about it, I completely agree that I shouldn't tell the user any more information than what is necessary.
I'll work on that.

dday9 without testing your code, do you think it will handle dbnull values.

From what I've read this is the line where the exception will be thrown if no rows are returned:

 'Use ExecuteScalar to return a single value
            count = Convert.ToInt32(cmd.ExecuteScalar())

But will it return a row if the [Password] field is null?

[techgnome]

That is initially what I suggested, but it appears as though the OP requires to store the username and PC name rather than simply confirming if a row exists with the desired username and PC name. Which now that I think about it, if the OP is entering in the username and PC name then he'd already have those two values... I think that I will re-suggest what I posted in post #2.

I thought about that too... but neither the PCName nor the UserName came from the data:

                    UserModule.PCName = txtPcName.Text
                    UserModule.UserName = txtUsername.Text

which led me back around to the scalar result.

-tg

[dday9]

This does not account for the password field parameter.

Add the password parameter to the function, correct the SQL, add the new parameter to the command:

Private Function RowCount(ByVal username As String, ByVal PcName As String, ByVal password As String)

    '...
    Using cmd As OleDbCommand = New OleDbCommand("SELECT Count([ID]) FROM [IDStaff] WHERE [StaffName]=@staff AND [PCNAME]=@pc AND [Password]=@pass", con)
        'Parameterize the query
        With cmd.Parameters
            .AddWithValue("@staff", username)
            .AddWithValue("@pc", pcName)
            .AddWithValue("@pass", password)
        End With

        '....

I guessing, no I'm sure "" is not the same as null, so it seems like the isDBNull check is appropriate here to check the field for null and cast it to a type if it is. This is what I think JMC was guiding me to do anyway.

Yes, an empty String is different from a Null value. The difference is that a String still contains a value if it is empty where as a Null value is the absence of a value altogether.

dday9 without testing your code, do you think it will handle dbnull values.

Absolutely.

I do like the idea of just returning a Boolean value

If you use the code that I provide then you'll still be able to use the function in a conditional statement:

If RowCount(txtUsername.Text, txtPcName.Text, txtPassword.Text) > 0 Then
    'The user exists
Else
    'Invalid login
End If

From what I've read this is the line where the exception will be thrown if no rows are returned:

Wrong, if no rows are returned then the count variable will be 0. The function will return a -1 if an exception occurs.

[techgnome]

I think you need to turn your reason and logic on its head... or yourself and look at things differently....

My problem was in the SQL statement, where the password could be a null string, either in the db or in as user input

As dday9 suggests:



If either the UserName and PcName differs from the user input then no row is returned.
This does not account for the password field parameter.



Was the original Select Statement, however if the password field is null then no row is returned due to the Select Statement looking for a password of "". <--Empty string in txtPassword textbox

Ungh... you shouldn't be using string concatenation... Parameters is where it's at. It's all the rage. Has been for the last 20 years. Cool thing about parameters - you can pass in a null value. With string concatenation, it's all string so it's harder to pass in the null value (as I think about it the term "null value" is an oxymoron, as null is the absence of a value, an "unknown" if you will). Plus there is also the Little bobby Drop Tables problem, which parameters will help with.

I guessing, no I'm sure "" is not the same as null, so it seems like the isDBNull check is appropriate here to check the field for null and cast it to a type if it is. This is what I think JMC was guiding me to do anyway.

Correct, and empty string IS NOT the same as a null value. Null is the absence of a value. It's possible that an empty string is in fact a valid value... while a null means there is no value, not even an empty string. If it helps, think of a null as being similar to nothing in VB (still not the same, but it helps illustrate the point.)

I do like the idea of just returning a Boolean value

Techgnome, after I think about it, I completely agree that I shouldn't tell the user any more information than what is necessary.
I'll work on that.

dday9 without testing your code, do you think it will handle dbnull values.

From what I've read this is the line where the exception will be thrown if no rows are returned:

 'Use ExecuteScalar to return a single value
            count = Convert.ToInt32(cmd.ExecuteScalar())

But will it return a row if the [Password] field is null?

ExecuteScalar will return a value or it will not... depends on the final SQL you employ. -- first WHY would you allow NULL for a password? -- at any rate, for NULLs you can't check to see if they are equal... but you can check if something IS null or not...

where (((SomeField IS NULL) and (@SomeParam is null)) or (SomeField = @SomeParam))) and (UserName = @UserName)

is probably how'd I approach it... unless I change the null passwords... but that's neither here nor there.

-tg

[crater]

 Public Function RowCount(ByVal username As String, ByVal pcName As String, ByVal Password As String) As Integer

        'Declare the object to return
        Dim count As Integer = -1

        'Declare the connection object
        Dim con As OleDbConnection

        'Wrap code in Try/Catch
        Try
            'Set the connection object to a new instance
            'TODO: Change "My Connection String Here" with a valid connection string
            con = New OleDbConnection(My.Settings.OneDataConnectionString)

            'Create a new instance of the command object
            'TODO: Change [ID] to a valid column and [MyTable] to a valid table
            Using cmd As OleDbCommand = New OleDbCommand("SELECT Count([15]) FROM [IDStaff] WHERE [StaffName]=@staff AND [PCNAME]=@pc AND [Password]=@Pass", con)
                'Parameterize the query
                With cmd.Parameters
                    .AddWithValue("@staff", username)
                    .AddWithValue("@pc", pcName)
                    .AddWithValue("@Pass", Password)
                End With

                'Open the connection
                con.Open()

                'Use ExecuteScalar to return a single value
                count = Convert.ToInt32(cmd.ExecuteScalar())

                'Close the connection
                con.Close()
            End Using
        Catch ex As Exception
            'Display the error
            Console.WriteLine(ex.Message)
        Finally
            'Check if the connection object was initialized
            If con IsNot Nothing Then
                If con.State = ConnectionState.Open Then
                    'Close the connection if it was left open(exception thrown)
                    con.Close()
                End If

                'Dispose of the connection object
                con.Dispose()
            End If
        End Try

        'Return the row count
        Return count

    End Function

This gives me an exception "No value given for one or more required parameters."
the exception happens here:

 'Use ExecuteScalar to return a single value
                count = Convert.ToInt32(cmd.ExecuteScalar())

This is what I thought would happen. Please don't take me wrong I certainly appreciate all the help. I would think regardless of what us returned the function should complete, but it doesn't.

Ideas now. And is there some kind of performance issue using a DataReader instead of ExecuteScalar?

[crater]

Sorry, you guys must feel like your wasting your time but,

1) I really do get the need for parameters, although it sure doesn't look like it i'd admit.
1) I don't get to choose weather to allow nulls as passwords, this is a database that was designed by others, and other software handles the password data.

2) I don't get :

where (((SomeField IS NULL) and (@SomeParam is null)) or (SomeField = @SomeParam))) and (UserName = @UserName)

is this how you you handle the select statement?

[dday9]

This is what I thought would happen. Please don't take me wrong I certainly appreciate all the help. I would think regardless of what us returned the function should complete, but it doesn't.

Don't blame me, you didn't use the code I provided

Your issue I believe lies in how you've changed the Column name in the SQL Count function.

[crater]

Don't blame me, you didn't use the code I provided

Ahh ha, damn-it batman they caught me.

It was indeed in the sql Count function.

Looks like I have some research to do now on DataReader and ExecuteScalar. I wish to learn the difference, since both can be made to do the same thing.

[crater]

Thanks to all who have gave input especially dday9, you rock.

[dday9]

With a DataReader it will read one row at a time. With a OleDbDataReader you have to use the object as a vehicle to do comparison on by calling the Read method which in turn returns a Boolean value, this is why you'll often see OleDbDataReaders in a Do/Until loop with the condition at the beginning:

Do Until Not MyDataReader.Read

Loop

'Or

Do While MyDataReader.Read

Loop

If there is a row that is returned then you can get the values based on the column name or index. In your situation since you only want to know if there is a Row you could just return the value of Read:

Return MyDataReader.Read

ExecuteScalar will only ever return a single value. The way that I structure my SQL command is that it will return the Count value from the amount of rows returned. Let's take a different example of where you want to get the username based on the ID value, then you'd use:

Private Function GetUsername(ByVal id As Integer) As String

    '...
    Dim user As String = String.Empty
    Using cmd As OleDbCommand = New OleDbCommand("SELECT [StaffName] FROM [IDStaff] WHERE [ID]=@id", con)
        'Parameterize the query
        cmd.Parameters.AddWithValue("@id", id)

        'Open the connection
        con.Open()

        'Use ExecuteScalar to return a single value
        user = cmd.ExecuteScalar().ToString()

        '...

    Return user
End Function

[crater]

So if I'm following correctly:

Use ExecuteScalar in query's where I only need to return a single value. Seems to me there has to be a reader somewhere but maybe that's getting too technical. Never-the-less, this must be faster than looping through all the rows.

AND

Use a OleDbDataReader in query's where I would want to return multipal values. This would be accomplished by looping.

[techgnome]

So if I'm following correctly:

Use ExecuteScalar in query's where I only need to return a single value. Seems to me there has to be a reader somewhere but maybe that's getting too technical.

You're counting screw threads when you don't need to. Behind the scenes it maybe using a stream of some kind to get the data... but that's so far down the technical line that it doesn't matter. It's like worrying about the car starting because you're sure there must be a spark plug in there somewhere - not all engines use a spark plug, some use a glow plug, which works differently, but it still works.
It's an implementation detail that the framework takes care of for you so you don't need to worry about it. It might be that today it uses a reader of some kind on the back end. Then one day someone discovers a faster method... and so the framework is re-wired to use this new method... because it is baked right in, you don't see the change, but get the benefit of it.

Never-the-less, this must be faster than looping through all the rows.

AND

Use a OleDbDataReader in query's where I would want to return multipal values. This would be accomplished by looping.

IT depends... there are multiple ways to get data for a reason... it might be that a reader isn't alwyas the best way. Sometimes you'll need to dump it into a datatable and then loop through that. then there's stored procedures and even parameterized quiesries that can have output parameters...

-tg

[crater]

You're counting screw threads when you don't need to. Behind the scenes it maybe using a stream of some kind to get the data... but that's so far down the technical line that it doesn't matter. It's like worrying about the car starting because you're sure there must be a spark plug in there somewhere - not all engines use a spark plug, some use a glow plug, which works differently, but it still works.
It's an implementation detail that the framework takes care of for you so you don't need to worry about it. It might be that today it uses a reader of some kind on the back end. Then one day someone discovers a faster method... and so the framework is re-wired to use this new method... because it is baked right in, you don't see the change, but get the benefit of it.

This is what I kind-of thought.

IT depends... there are multiple ways to get data for a reason... it might be that a reader isn't alwyas the best way. Sometimes you'll need to dump it into a datatable and then loop through that. then there's stored procedures and even parameterized quiesries that can have output parameters...

This is my own challenge: To separate the ways of getting data based on the reason or logic.
I have become very accustom to using strongly typed datasets, data adapters and bindingsources that are produced via the datasource wizard, sadly without giving much thought of the "built-in functions these assembly's provide.

Thanks for the info

[techgnome]

Here's how I grab data from the database... again, note there is no explicit reader here...

    Private Function GetColumnNames(dbConn As SqlClient.SqlConnection, ImportProcessID As Guid) As DataTable
        Dim retDataTable As New DataTable
        Dim sqlAdapter As New SqlClient.SqlDataAdapter
        Dim sqlCommand As New SqlClient.SqlCommand()
        sqlCommand.Connection = dbConn
        sqlCommand.CommandType = CommandType.StoredProcedure
        sqlCommand.CommandText = "usp_ExtractTableData"
        sqlCommand.Parameters.AddWithValue("PROCESSID", ImportProcessID)
        sqlAdapter.SelectCommand = sqlCommand
        sqlAdapter.Fill(retDataTable)
        Return retDataTable
    End Function

This is fairly typical of how I'll call a stored proc that returns a bunch of data... I dump it into a datatable, then return that... from there I work disconnected and can loop through the data. In fact, rigght after I call this funciton, I call another one almost exactly like it to get some additional data from elsewhere... I then take both of those dataTables and pass them to yet another function that does some UI updating. Readers in this case would not work for me.

-tg