Automatic variable generation from data table

Question

I am adjusting Fast Report to create some reports, and I have created a loop that runs through query results stored inside a datatable and assigns each TenantName to a variable. However, the code I wrote seems bulky and would get out of hand if I needed more variables.

Would there be a more sensible way to do this?

If cmbReports.Text = "Conditions Of Tenancy" Then
        Dim ListReport = New FastReport.Report
        ListReport.Load("C:\Users\richard\Documents\BMSSouthSide\BMSPanda\Reports\CONDITIONSOFTENANCY.frx")
        ListReport.SetParameterValue("CRMConnectionString", "Data ..... ")
        Dim strSql As String = "Select  TenantForename + ' ' + TenantSurname as FullName From Tenants Where '" & lblLeaseIDValue.Text & "' = LeaseID and PropertyID =" & lblPropertyIDValue.Text
        Dim dtb As New DataTable
        Dim ten1 As String
        Dim ten2 As String
        Dim ten3 As String
        Dim ten4 As String
        Dim ten5 As String
        Dim ten6 As String
        Dim ten7 As String
        Dim ten8 As String
        Dim ten9 As String
        Dim ten10 As String
        Dim ten11 As String
        Dim IntTenantID As Integer
        Using cnn As New SqlConnection(My.Settings.BMSSouthSideConnectionString)
            cnn.Open()
            Using dad As New SqlDataAdapter(strSql, cnn)
                dad.Fill(dtb)
            End Using
            Dim Ten As Integer = dtb.Rows.Count
            Do Until Ten = 0
                For Each row As DataRow In dtb.Rows
                    If Ten = 11 Then Ten1 = row("FullName")
                    If Ten = 10 Then Ten2 = row("FullName")
                    If Ten = 9 Then Ten3 = row("FullName")
                    If Ten = 8 Then Ten4 = row("FullName")
                    If Ten = 7 Then Ten5 = row("FullName")
                    If Ten = 6 Then Ten6 = row("FullName")
                    If Ten = 5 Then Ten7 = row("FullName")
                    If Ten = 4 Then Ten8 = row("FullName")
                    If Ten = 3 Then Ten9 = row("FullName")
                    If Ten = 2 Then Ten10 = row("FullName")
                    If Ten = 1 Then Ten11 = row("FullName")
                    Ten = Ten - 1T
                Next row
            Loop
            Dim strSql2 As String = "Select Top 1 TenantID From Tenants Where '" & lblLeaseIDValue.Text & "' = LeaseID and PropertyID =" & lblPropertyIDValue.Text
            Dim datatableTenantID As New DataTable
            Using Tenant As New SqlDataAdapter(strSql2, cnn)
                Tenant.Fill(datatableTenantID)
            End Using
            For Each IntTenant As DataRow In datatableTenantID.Rows
                IntTenantID = IntTenant("TenantID")
            Next
            cnn.Close()
        End Using
        Dim TenantList As String = Ten11 & " " & Ten10 & " " & Ten9 & " " & Ten8 & " " & Ten7 & " " & Ten6 & " " & Ten5 & " " & Ten4 & " " & Ten3 & " " & Ten2 & " " & Ten1
        ListReport.SetParameterValue("PropertyID", Val(lblPropertyIDValue.Text))
        ListReport.SetParameterValue("LeaseID", Val(lblLeaseIDValue.Text))
        ListReport.SetParameterValue("TenantID", IntTenantID)
        ListReport.SetParameterValue("TenantList", TenantList)
        ListReport.Show()
    End If

Answer 1

• Use array, this will reduce by a lot your number of variables.
• Name your variable properly ("ten" should be "tenant")
• Normaly, variable starts with a lower case in .net
• Do not concatenate your strings for queries, this will increase your chances of injections
• Your datatable is a bit useless in your case, you can write directly to your variables with a datareader
• Your listReport is only needed at the end, I think you could initialize it there instead
• I would separate this into multiple functions. A "GetTenants" that returns the list, a "GetTenantID" and a "ShowReport". This way these function can be out of the UI and reused if needed.

• Your report takes multiple tenant names but only one ID, this seems strange.

  If cmbReports.Text = "Conditions Of Tenancy" Then
  
      Dim tenants As New List(Of String)
      Dim tenantID As Integer
  
      Dim propertyId As Integer
      Dim leaseId As Integer
  
      If Not Int32.TryParse(lblPropertyIDValue.Text, propertyId) Then
          ' Not an integer, this is an error
      End If
  
      If Not Int32.TryParse(lblLeaseIDValue.Text, leaseId) Then
          ' Not an integer, this is an error
      End If
  
      tenants = GetTenants(propertyId, leaseId)
      tenantID = GetTenantID(propertyId, leaseId)
  
      ShowReport(tenants, tenantID, propertyId, leaseId)
  
  End If
  
  Function GetTenants(ByVal propertyId As Integer, ByVal leaseId As Integer) As List(Of String)
  
      Dim tenants As New List(Of String)
  
      Using cnn As New SqlConnection(My.Settings.BMSSouthSideConnectionString)
          cnn.Open()
  
          Dim command As New SqlCommand("Select  TenantForename + ' ' + TenantSurname as FullName From Tenants Where LeaseID = @LeaseID and PropertyID = @PropertyID", cnn)
  
          command.Parameters.Add("@LeaseID", SqlDbType.Int).Value = leaseId
          command.Parameters.Add("@PropertyID", SqlDbType.Int).Value = propertyId
  
          Using reader As command.ExecuteReader()
              Do While reader.Read()
                  tenants.Add(reader.GetString("FullName"))
              Loop
          End Using
  
          cnn.Close()
      End Using
  
      Return tenants
  End Function
  
  Function GetTenantID(ByVal propertyId As Integer, ByVal leaseId As Integer) As Integer
  
      Dim tenantID As Integer
  
      Using cnn As New SqlConnection(My.Settings.BMSSouthSideConnectionString)
          cnn.Open()
  
          Dim command As New SqlCommand("Select Top 1 TenantID From Tenants Where LeaseID = @LeaseID and PropertyID = @PropertyID", cnn)
  
          command.Parameters.Add("@LeaseID", SqlDbType.Int).Value = leaseId
          command.Parameters.Add("@PropertyID", SqlDbType.Int).Value = propertyId
  
          Using reader As command.ExecuteReader()
              If reader.Read() Then
                  tenantID = reader.GetInt32("TenantID")
              End If
          End Using
  
          cnn.Close()
      End Using
  
      Return tenantID
  End Function
  
  Sub ShowReport(ByVal tenants As List(Of String), ByVal tenantID As Integer, ByVal propertyId As Integer, ByVal leaseId As Integer)
  
      Dim listReport = New FastReport.Report
  
      listReport.Load("C:\Users\richard\Documents\BMSSouthSide\BMSPanda\Reports\CONDITIONSOFTENANCY.frx")
      listReport.SetParameterValue("CRMConnectionString", "Data .....
  
      Dim tenantList As String = String.Join(" ", tenants.ToArray())
  
      ListReport.SetParameterValue("PropertyID", propertyId)
      ListReport.SetParameterValue("LeaseID", tenantID)
      ListReport.SetParameterValue("TenantID", tenantID)
      ListReport.SetParameterValue("TenantList", tenantList)
      ListReport.Show()
  
  End Sub
  

Answer 2

Couple of things here.

• A DataTable also makes use of the IDisposable Interface so you should use a using block with that as well
• no need to explicitly close your connection because the IDisposable Interface does that in the Using statement

---

It looks like you have one of your Where conditions backwards, I am going to assume that for the rest of this review and turn it around.

I don't understand why you would write this

Where '" & lblLeaseIDValue.Text & '" = LeaseID

This doesn't look right to me. Are you allowing the user to input the column name of the Where condition? like I said, I assume that this is backwards for the rest of this review.

---

Also, your Select statement parameters should be actual parameters I will show you in a second.

It is better to use a SqlCommand and insert your Select statement with parameters and then perform an ExecuteNonQuery and pass the Command to the DataAdapter. This way you have parameterized your inputs, making them a little safer against Sql Injection.

Here is what I came up with without breaking this code up into Methods/Functions, which you should do like @the_lotus says to do, but with more Using statements

If cmbReports.Text = "Conditions Of Tenancy" Then
    Dim ListReport = New FastReport.Report
    ListReport.Load("C:\Users\richard\Documents\BMSSouthSide\BMSPanda\Reports\CONDITIONSOFTENANCY.frx")
    ListReport.SetParameterValue("CRMConnectionString", "Data ..... ")
    Dim dtb As New DataTable
    Dim ten1 As String
    Dim ten2 As String
    Dim ten3 As String
    Dim ten4 As String
    Dim ten5 As String
    Dim ten6 As String
    Dim ten7 As String
    Dim ten8 As String
    Dim ten9 As String
    Dim ten10 As String
    Dim ten11 As String
    Dim IntTenantID As Integer
    Dim strSql As String = "Select  TenantForename + ' ' + TenantSurname as FullName From Tenants Where LeaseID = @lblLeaseIDValue and PropertyID = @lblPropertyIDValue"

    Using cnn As New SqlConnection(My.Settings.BMSSouthSideConnectionString)
        Using cmd1 As New SqlCommand(strSql, cnn)
            cmd1.Parameters.AddWithValue("@lblLeaseIDValue", lblLeaseIDValue.Text)
            cmd1.Parameters.AddWithValue("@lblPropertyIDValue", lblPropertyValueID.Text)
            cmd1.ExecuteNonQuery()
            Using dad As New SqlDataAdapter(cmd1, cnn)
                Using dtb As New DataTable()
                    cnn.Open()
                    dad.Fill(dtb)
                    Dim Ten As Integer = dtb.Rows.Count
                    Do Until Ten = 0
                        For Each row As DataRow In dtb.Rows
                            If Ten = 11 Then Ten1 = row("FullName")
                            If Ten = 10 Then Ten2 = row("FullName")
                            If Ten = 9 Then Ten3 = row("FullName")
                            If Ten = 8 Then Ten4 = row("FullName")
                            If Ten = 7 Then Ten5 = row("FullName")
                            If Ten = 6 Then Ten6 = row("FullName")
                            If Ten = 5 Then Ten7 = row("FullName")
                            If Ten = 4 Then Ten8 = row("FullName")
                            If Ten = 3 Then Ten9 = row("FullName")
                            If Ten = 2 Then Ten10 = row("FullName")
                            If Ten = 1 Then Ten11 = row("FullName")
                            Ten = Ten - 1T
                        Next row
                    Loop
                End Using
            End Using
        End Using


        Dim strSql2 As String = "Select Top 1 TenantID From Tenants Where LeaseID = @lblLeaseIDValue and PropertyID = @lblPropertyIDValue"
        Using datatableTenantID As New DataTable
            Using cmd As New SqlCommand(strSql2, cnn)
                cmd.Parameters.AddWithValue("@lblLeaseIDValue", lblLeaseIDValue.Text)
                cmd.Parameters.AddWithValue("@lblPropertyIDValue", lblPropertyValueID.Text)
                cmd.ExecuteNonQuery()
                Using Tenant As New SqlDataAdapter(cmd, cnn)
                    Tenant.Fill(datatableTenantID)
                    For Each IntTenant As DataRow In datatableTenantID.Rows
                        IntTenantID = IntTenant("TenantID")
                    Next
                    cnn.Close()
                End Using
            End Using
        End Using
    End Using
    Dim TenantList As String = Ten11 & " " & Ten10 & " " & Ten9 & " " & Ten8 & " " & Ten7 & " " & Ten6 & " " & Ten5 & " " & Ten4 & " " & Ten3 & " " & Ten2 & " " & Ten1
    ListReport.SetParameterValue("PropertyID", Val(lblPropertyIDValue.Text))
    ListReport.SetParameterValue("LeaseID", Val(lblLeaseIDValue.Text))
    ListReport.SetParameterValue("TenantID", IntTenantID)
    ListReport.SetParameterValue("TenantList", TenantList)
    ListReport.Show()
End If

---

Your loop oddly resembles a for loop I am sure that if you were to use some other variable types for your tenant list that you could clean that loop up quite a bit.

Answer 3

You can generate your query using dynamic sql. With only 11 tenants, it doesn't make the code that much shorter, but it makes it extendable to however many tenants you want.

declare @minten int;
declare @maxten int;
declare @query varchar(8000);
declare @dims varchar(8000);
declare @ifs varchar(8000);
declare @TenList varchar(8000);
set @minten = 1;
set @maxten = 11;

with tmp(t) as (
    select @minten
    union all
    select t+1
    from tmp
    where t< @maxten)
Select * into #tmp from tmp

Select @dims = coalesce(@dims + 'dim Ten' + cast(t as varchar(3)) + ' as String
', 'dim Ten' + cast(t as varchar(3)) + ' as String
') from #tmp
order by t
Select @ifs = coalesce(@ifs + 'If Ten = ' + cast(t as varchar) + ' Then Ten' + cast(t as varchar(3)) + ' = row("FullName")
','If Ten = ' + cast(t as varchar) + ' Then Ten' + cast(t as varchar(3)) + ' = row("FullName")
') from #tmp
order by t
Select @TenList = coalesce(@TenList + ' & " " & Ten' + cast(t as varchar(3)),'Ten' + cast(t as varchar(3)))
from #tmp
order by t


set @query = 'If cmbReports.Text = "Conditions Of Tenancy" Then
        Dim ListReport = New FastReport.Report
        ListReport.Load("C:\Users\richard\Documents\BMSSouthSide\BMSPanda\Reports\CONDITIONSOFTENANCY.frx")
        ListReport.SetParameterValue("CRMConnectionString", "Data .....
        Dim strSql As String = "Select  TenantForename + '' '' + TenantSurname as FullName From Tenants Where ''" & lblLeaseIDValue.Text & "'' = LeaseID and PropertyID =" & lblPropertyIDValue.Text
        Dim dtb As New DataTable
        ' + @dims + '
        Dim IntTenantID As Integer
        Using cnn As New SqlConnection(My.Settings.BMSSouthSideConnectionString)
            cnn.Open()
            Using dad As New SqlDataAdapter(strSql, cnn)
                dad.Fill(dtb)
            End Using
            Dim Ten As Integer = dtb.Rows.Count
            Do Until Ten = 0
                For Each row As DataRow In dtb.Rows
        ' + @ifs + '
        Ten = Ten - 1T
                Next row
            Loop
            Dim strSql2 As String = "Select Top 1 TenantID From Tenants Where ''" & lblLeaseIDValue.Text & "'' = LeaseID and PropertyID =" & lblPropertyIDValue.Text
            Dim datatableTenantID As New DataTable
            Using Tenant As New SqlDataAdapter(strSql2, cnn)
                Tenant.Fill(datatableTenantID)
            End Using
            For Each IntTenant As DataRow In datatableTenantID.Rows
                IntTenantID = IntTenant("TenantID")
            Next
            cnn.Close()
        End Using
        Dim TenantList As String = ' + @TenList + '
        ListReport.SetParameterValue("PropertyID", Val(lblPropertyIDValue.Text))
        ListReport.SetParameterValue("LeaseID", Val(lblLeaseIDValue.Text))
        ListReport.SetParameterValue("TenantID", IntTenantID)
        ListReport.SetParameterValue("TenantList", TenantList)
        ListReport.Show()
    End If'

    Select @query
    --Execute (@query)