CODEWITHSUNDEEP
vb.netoptimization
ADY
ADY

Reputation: 113

Is there a better looping mechanism for this code that won't throw a code analysis warning?

I recently upgraded to VB.NET 2019 and have been going through code analysis to clean everything up in a database program to what MS considers "best practice". This bit of code and a couple just like it I'm having a issue with. The analysis says "Value assigned to 'x' is never used" which is true but I'm not sure of a better way of writing it. Long story short a user can select multiple items in a gridview then run a report and the query is built based on that using or statements. It runs through what is selected and adds to the query. If only one item is selected it just does the selected one (x=0) and if more are selected its adding a OR then the info. Code is cleaned up for simplicity:

Dim x As Integer = 0
For x = 0 To myGridView.SelectedRowsCount - 1
    If x = 0 Then
        mySQL += "{" & myQuery & ".CompanyID} =" & CInt(myGridView.SelectedRows(x))
    Else
        mySQL += " OR {" & myQuery & ".CompanyID} =" & CInt(myGridView.SelectedRows(x))
    End If
Next

Is there a better way to do this so it doesn't throw that error?

Upvotes: 0

Views: 68

Answers (2)

the_lotus
the_lotus

Reputation: 12748

Your actual warning might be because you initialize the integer but then use it in the For loop. Just initialize it in the For loop.

' Remove this line: Dim x As Integer = 0

For x As Integer = 0 To myGridView.SelectedRowsCount - 1
    ' ...
Next

Upvotes: 1

David
David

Reputation: 6111

You would be better off using a conditional statement as well as SQL's IN operator. Basically you'd check if myGridView.SelectedRowsCount equals 0, if so then you'd display an error. Then you'd have an ElseIf that checks if myGridView.SelectedRowsCount equals 1, if so then you'd use the equals operator. Finally you'd have an Else statement that joins the values using a comma as the glue.

Take a look at this example:

If myGridView.SelectedRowsCount = 0 Then
    MessageBox.Show("There are no rows selected. Please select one or more rows.", "Error", MessageBoxButtons.Ok, MessageBoxIcon.Error)
ElseIf myGridView.SelectedRowsCount = 1 Then
    mySQL &= "{" & myQuery & ".CompanyID} = " myGridView.SelectedRows(0)
Else
    mySQL &= "{" & myQuery & ".CompanyID} IN (" & String.Join(", ", myGridView.SelectedRows) & ")"
End If

Please keep in mind that this is pseudo-code and you may need to make adjustments, in particular I don't know if SelectedRows or SelectedRows(0) returns a String array/literal.

P.S. You may want to consider using a parameterized query. It would not be difficult to implement one using the same concept that I'm using above.

Upvotes: 2

Related Questions