trjhtr

I'm learning VB.NET for work reasons, and I'd like to ask your help to improve my coding and my programming skills. What can I do better?

The program will be used to store data in the DB, like how many pencil, rubber, etc. You have to store and update these values according to buying and selling orders.

This is my SQLTools class:

Imports System.Data.SqlClient
Imports System.Data

Public Class SQLTools
 Inherits System.Windows.Forms.Form
 Private Const SqlString As String = "Server=*****;Database=*****;User ID=*****;Password=*****;Integrated Security=SSPI;"
 Private myConn As SqlConnection
 Private myCmd As SqlCommand
 Private myReader As SqlDataReader

 Private Sub Conn()
 myConn = New SqlConnection(SqlString)
 End Sub

 Public Function Connect() As SqlConnection
 myConn = New SqlConnection(SqlString)
 Return myConn
 End Function

 Public Function GetValDouble(ByVal query As String) As String
 On Error GoTo Handler

 //Setting and opening connection
 Conn()
 myConn.Open()

 //Get any Double value from DB
 Dim Res As Integer
 Dim I = 0
 myCmd = myConn.CreateCommand
 myCmd.CommandText = Trim(query)
 myReader = myCmd.ExecuteReader()
 Do While myReader.Read()
 Res = Convert.ToString(myReader.GetValue(I))
 I += 1
 Loop
 myReader.Close()
 myConn.Close()
 Return Res

 Handler:
 MsgBox("Qualcosa è andato storto :(")
 Err.Clear()
 End Function

 Public Function GetValStr(ByVal query As String) As String
 On Error GoTo Handler

 //Setting and opening connection
 Conn()
 myConn.Open()

 //Get any string value from DB
 Dim Res As Integer
 Dim I = 0
 myCmd = myConn.CreateCommand
 myCmd.CommandText = Trim(query)
 myReader = myCmd.ExecuteReader()
 Do While myReader.Read()
 Res = Convert.ToString(myReader.GetValue(I))
 I += 1
 Loop
 myReader.Close()
 myConn.Close()
 Return Res

 Handler:
 MsgBox("Qualcosa è andato storto :(")
 Err.Clear()
End Function

Public Function GetValInt(ByVal query As String) As Integer
 On Error GoTo Handler

 //Setting and opening connection
 Conn()
 myConn.Open()

 //Get any int value from DB
 Dim Res As Integer
 Dim I = 0
 myCmd = myConn.CreateCommand
 myCmd.CommandText = Trim(query)
 myReader = myCmd.ExecuteReader()
 Do While myReader.Read()
 Res = Convert.ToInt16(myReader.GetValue(I))
 I += 1
 Loop
 myReader.Close()
 myConn.Close()
 Return Res

 Handler:
 MsgBox("Qualcosa è andato storto :(")
 Err.Clear()
End Function

Public Sub Delete(ByVal Table As String)
 //Opening connection
 Conn()
 myConn.Open()

 //Clearing transactions History
 Dim Sql As String = "Delete from " + Table
 myCmd = New SqlCommand(Sql, myConn)
 myCmd.ExecuteNonQuery()
 MsgBox("Cronologia ordini canellata.")
 myConn.Close()

End Sub

End Class

Insert Sell Order:

Imports System.Data.SqlClient
Imports System.Data

Public Class INS_Sell
 Inherits System.Windows.Forms.Form
 Private myConn As SqlConnection
 Private myCmd As SqlCommand
 Private myReader As SqlDataReader
 Dim Tool As New SQLTools()

Private Sub CLS_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles CLS.Click, CLS.Click
 Dim M As New Main
 M.Show()
 Me.Hide()
End Sub

Private Sub Exec_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Exec.Click
 On Error GoTo Handler

 //Opening DB connection
 Tool.Connect()
 myConn.Open()

 //Extrapolating value from textbox
 Dim Quant As Integer = Convert.ToInt16(TextBox1.Text)

 //Setting Quant to -Quant in order to sell Articles
 Quant -= Quant * 2

 //Selecting Articles from ComboBox
 Dim Articolo As String = ComboBox1.SelectedItem
 Dim Art As Integer
 Select Case Articolo
 Case "Quaderno"
 Art = 1
 Case "Penna"
 Art = 2
 Case "Matita"
 Art = 3
 Case "Gomma"
 Art = 4
 End Select

 //Quantity Checking for the choosen article
 If Tool.GetValInt("Select Quant from Magazzino where ID_Art=" + Convert.ToString(Art)) + Quant <= 0 Then
 MsgBox("Quantità insufficente, anullo ordine.")
 Exit Sub
 End If

 //Updating transactions chronology
 Dim Sql As String = "insert into Movimento values(@Art,@Quant)"
 myCmd = New SqlCommand(Sql, myConn)
 myCmd.Parameters.Add("@Art", SqlDbType.Int).Value = Art
 myCmd.Parameters.Add("@Quant", SqlDbType.Int).Value = Quant
 myCmd.ExecuteNonQuery()

 //Quantity Check for the transaction
 Quant = Tool.GetValInt("Select Quant from Magazzino where ID_Art=" + Convert.ToString(Art)) + Quant
 If Quant <= 0 Then
 Quant = 0

 //Autobuy article 
 ElseIf Quant <= 10 Then
 Quant += 20
 Sql = "Insert into Movimento values(@Art, 20)"
 myCmd = New SqlCommand(Sql, myConn)
 myCmd.Parameters.Add("@Art", SqlDbType.Int).Value = Art
 myCmd.ExecuteNonQuery()
 MsgBox("Quantità articolo sotto la soglia minima, acquistati 20 pezzi.")
 End If

 //Warehouse updating
 Sql = "update Magazzino set Quant = @Quant Where ID_Art = @Art"
 myCmd = New SqlCommand(Sql, myConn)
 myCmd.Parameters.Add("@Art", SqlDbType.Int).Value = Art
 myCmd.Parameters.Add("@QUant", SqlDbType.Int).Value = Quant
 myCmd.ExecuteNonQuery()
 MsgBox("Modifica avvenuta con successo!")
 myConn.Close()
 Exit Sub

 Handler:
 MsgBox("Qualcosa è andato storto :(")
 TextBox1.Text = ""
 End Sub

 Private Sub QRY_FormClosing(ByVal sender As Object, ByVal e As System.Windows.Forms.FormClosingEventArgs) Handles Me.FormClosing
 Application.Exit()
 End Sub

 Private Sub Form2_Load(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles MyBase.Load
 On Error GoTo Handler
 Dim Res As String = ""

 //Opening DB connection
 myConn = Tool.Connect()
 myConn.Open()

 //Populating Articles ComboBox
 myCmd = myConn.CreateCommand
 myCmd.CommandText = "Select Articolo From Magazzino"
 myReader = myCmd.ExecuteReader()
 Do While myReader.Read()
 For I = 0 To myReader.FieldCount() - 1
 Res = myReader.GetString(I)
 ComboBox1.Items.Add(Res)
 Next
 Loop
 myConn.Close()
 myReader.Close()
 Res = ""
 Exit Sub

 Handler:
 MsgBox("Impossibile recuperare i dati dal magazzino.")
 myConn.Close()
 Res = ""
 Err.Clear()

 End Sub
End Class

Insert Buy Order:

Imports System.Data.SqlClient
Imports System.Data

Public Class INS_Buy
 Inherits System.Windows.Forms.Form
 Private myConn As SqlConnection
 Private myCmd As SqlCommand
 Private myReader As SqlDataReader
 Dim Tool As New SQLTools()

Private Sub CLS_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles CLS.Click, CLS.Click
 Dim M As New Main
 M.Show()
 Me.Hide()
End Sub

Private Sub Exec_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Exec.Click
 On Error GoTo Handler

 //Opening DB connection
 myConn = Tool.Connect()
 myConn.Open()

 //Extrapolating value from textbox
 Dim Quant As Integer = Convert.ToInt16(TextBox1.Text)

 //Selecting Articles from ComboBox
 Dim Articolo As String = ComboBox1.SelectedItem
 Dim Art As Integer
 Select Case Articolo
 Case "Quaderno"
 Art = 1
 Case "Penna"
 Art = 2
 Case "Matita"
 Art = 3
 Case "Gomma"
 Art = 4
 End Select

 //Updating transactions chronology
 Dim Sql As String = "insert into Movimento values(@Art,@Quant)"
 myCmd = New SqlCommand(Sql, myConn)
 myCmd.Parameters.Add("@Art", SqlDbType.Int).Value = Art
 myCmd.Parameters.Add("@Quant", SqlDbType.Int).Value = Quant
 myCmd.ExecuteNonQuery()

 //Warehouse updating
 Sql = "update Magazzino set Quant = @Quant Where ID_Art = @Art"
 myCmd = New SqlCommand(Sql, myConn)
 myCmd.Parameters.Add("@Art", SqlDbType.Int).Value = Art
 myCmd.Parameters.Add("@QUant", SqlDbType.Int).Value = Tool.GetValInt("Select Quant from Magazzino where ID_Art=" + Convert.ToString(Art)) + Quant
 myCmd.ExecuteNonQuery()
 MsgBox("Modifica avvenuta con successo!")
 myConn.Close()
 Exit Sub

 Handler:
 MsgBox("Qualcosa è andato storto :(")
 TextBox1.Text = ""
 End Sub

Private Sub QRY_FormClosing(ByVal sender As Object, ByVal e As System.Windows.Forms.FormClosingEventArgs) Handles Me.FormClosing
 Application.Exit()
End Sub

Private Sub Form2_Load(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles MyBase.Load
 On Error GoTo Handler
 Dim Res As String = ""

 //Opening DB connection
 myConn = Tool.Connect()
 myConn.Open()

 //Populating Articles ComboBox
 myCmd = myConn.CreateCommand
 myCmd.CommandText = "Select Articolo From Magazzino"
 myReader = myCmd.ExecuteReader()
 Do While myReader.Read()
 For I = 0 To myReader.FieldCount() - 1
 Res = myReader.GetString(I)
 ComboBox1.Items.Add(Res)
 Next
 Loop
 myConn.Close()
 myReader.Close()
 Res = ""
 Exit Sub

 Handler:
 MsgBox("Impossibile recuperare i dati dal magazzino.")
 myConn.Close()
 Res = ""
 Err.Clear()
 End Sub
End Class

SQLTools

Insert Buy Order

Insert Sell Order

so first thing I would say to you is

1. Don't use Goto's

Second thing

2. Don't swallow Exceptions

You also are using C# comment indicators when you should be using an apostrophe

to illustrate some of what I am saying I grabbed your Form2 Load Sub from the Insert and got rid of the goto exception handling. I also put in 2 using statements that will automatically dispose of the Connection and the Reader upon leaving scope of the Using block.

Check it out:

Yours

Private Sub Form2_Load(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles MyBase.Load
 On Error GoTo Handler
 Dim Res As String = ""

 //Opening DB connection
 myConn = Tool.Connect()
 myConn.Open()

//Populating Articles ComboBox
 myCmd = myConn.CreateCommand
 myCmd.CommandText = "Select Articolo From Magazzino"
 myReader = myCmd.ExecuteReader()
 Do While myReader.Read()
 For I = 0 To myReader.FieldCount() - 1
 Res = myReader.GetString(I)
 ComboBox1.Items.Add(Res)
 Next
 Loop
 myConn.Close()
 myReader.Close()
 Res = ""
 Exit Sub

Handler:
 MsgBox("Impossibile recuperare i dati dal magazzino.")
 myConn.Close()
 Res = ""
 Err.Clear()

End Sub

Mine

Private Sub From1_Load(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles MyBase.Load
 Dim res As String = ""
 Using myConn As SqlConnection = Tool.Connect()
 myConn.Open()
 myCmd = myConn.CreateCommand
 myCmd.CommandText = "Select Articolo From Magazzino"
 Using myReader As SqlDataReader = myCmd.ExecuteReader()
 Do While myReader.Read()
 For I = 0 To myReader.FieldCount() - 1
 res = myReader.GetString(I)
 ComboBox1.Items.Add(res)
 Next
 Loop
 End Using
 End Using
End Sub

for doing your Error Handling. if you have to catch the exceptions and handle them the way you were, you should use a try-catch-finally block. like this

 Try
 Dim res As String = ""
 Using myConn As SqlConnection = Tool.Connect()
 myConn.Open()
 myCmd = myConn.CreateCommand
 myCmd.CommandText = "Select Articolo From Magazzino"
 Using myReader As SqlDataReader = myCmd.ExecuteReader()
 Do While myReader.Read()
 For I = 0 To myReader.FieldCount() - 1
 res = myReader.GetString(I)
 ComboBox1.Items.Add(res)
 Next
 Loop
 End Using
 End Using
 Catch ex As Exception
 MsgBox("Impossibile recuperare i dati dal magazzino.")
 Finally
 Res = ""
 Err.Clear()
 End Try
End Sub

not sure why you would need to set that Res variable to an empty string, it will be Garbage Collected after the scope leaves this Sub regardless.

Notice how I didn't close the connection or the reader, this is because the Using blocks are actually Try Finally statements where in the Finally statement they call the IDispose's dispose method for the resource that was being used. so the connection is disposed once the scope leaves that block, same with the reader.

Error Handling and Using blocks have already been covered by @Malachi. My comments are inline.

Option Strict On
'Always and forever set Option Strict On
'This can be set for all your VB projects
Imports System.Data.SqlClient
 Imports System.Data

 Public Class SQLTools
 'Inherits System.Windows.Forms.Form
 'Don't inherit form Windows.Forms - this is Not a Form, it is a helper class
 Private Const SqlString As String = "Server=*****;Database=*****;User ID=*****;Password=*****;Integrated Security=SSPI;"
 Private myConn As SqlConnection
 Private myCmd As SqlCommand
 Private myReader As SqlDataReader

 Private Sub Conn()
 myConn = New SqlConnection(SqlString)
 End Sub

 Public Function GetConnectionObject() As SqlConnection
 myConn = New SqlConnection(SqlString)
 Return myConn
 End Function
 'Your Function is called GetValDouble but it returns a String
 'this is very misleading to anyone trying to use this class
 Public Function GetValDouble(ByVal query As String) As String
 'Your use of GoTo has already been covered
 On Error GoTo Handler

 'Setting And opening connection
 Conn() 'bad name for a Sub - What does it do?
 'How about calling it CreateNewConnection
 myConn.Open()

 'Get any Double value from DB
 Dim Res As Integer 'Your function is defined to return an String 
 'but you are trying to return an Integer - won't compile
 Dim I = 0
 myCmd = myConn.CreateCommand
 myCmd.CommandText = Trim(query)
 myReader = myCmd.ExecuteReader()
 'Suppose the passed in query returns 100 records
 'I is being incremented on each iteration
 'the .GetValue(I) is trying to read the column(I) in the return
 'Are you returning 100 columns?
 Do While myReader.Read()
 'Now you are trying to assign a string to the variable
 'that you just declared as an integer, yet your Function
 'title is promising a Double!
 Res = Convert.ToString(myReader.GetValue(I))
 'Assuming the assignment could be made, you are overwritting the
 'the value of Res on each iteration
 I += 1
 Loop
 myReader.Close()
 myConn.Close()
 Return Res

Handler:
 'Don't communicate with the user from a helper class
 'Let the error go up the call stack until you handle it in a UI class (a Form)
 MsgBox("Qualcosa è andato storto :(")
 Err.Clear()
 End Function