First, you don't open the connection:
con.Open()
Next, password
is a reserved word in MS Access. You would need to wrap password
in square brackets:
[password]
You are concatenating strings instead of using paramaters:
cmd.Parameters.Add("@username", OleDbType.VarChar).Value = txtUsername.Text
cmd.Parameters.Add("@password", OleDbType.VarChar).Value = txtPassword.Text
cmd.Parameters.Add("@typeofuser", OleDbType.VarChar).Value = cmbTypeOfUser.Text
Look at giving your TextBox
and ComboBox
controls a proper name instead of using TextBox1
, TextBox2
and ComboBox1
. This helps to identify correctly each control:
txtUsername
txtPassword
cmbTypeOfUser
Move away from using MsgBox
and use MessageBox.Show
. MsgBox
exists for VB6 and ends up delegating to MessageBox
anyway so makes sense to use MessageBox.Show
:
MessageBox.Show("No Username and Password inserted")
Lastly I would consider implementing Using which will help to close and dispose of your SQL objects:
Using cmd As New OleDbCommand(command, connection)
End Using
All together your code would look something like this:
If txtUsername.Text = Nothing And txtPassword.Text = Nothing Then
MessageBox.Show("No Username and Password inserted")
TextBox1.Focus()
Else
Using con As New OleDbConnection(connectionString),
cmd As New OleDbCommand("INSERT INTO [loginTable] ([username], [password], [typeofuser]) VALUES (@username, @password, @typeofuser)", con)
con.Open()
cmd.Parameters.Add("@username", OleDbType.VarChar).Value = txtUsername.Text
cmd.Parameters.Add("@password", OleDbType.VarChar).Value = txtPassword.Text
cmd.Parameters.Add("@typeofuser", OleDbType.VarChar).Value = cmbTypeOfUser.Text
cmd.ExecuteNonQuery()
End Using
End If
It's outside the scope of this question but I would also look at encrypting passwords. Storing them as plain text is bad practice. Have a look at the SO question; Best way to store password in database, which may give you some ideas on how best to do this.