0

I have a VB.NET method, which connects to a database and reads the value. And this value is used multiple times in multiple places. The method looks like below:

Public Function IsConfigurationEnabled() As Integer

    Dim IsEnabled As Integer
    Dim sqlText As String
    sqlText = "select value from [dbo].[Settings] where Configuration='XXX'"
    Dim connection As SqlConnection = New SqlConnection()
    Dim cmd As SqlCommand
    connection.ConnectionString = "Data Source=localhost;Initial Catalog=XXX;Integrated Security=True"
    Try
        connection.Open()
        cmd = New SqlCommand(sqlText, connection)
        IsEnabled = Convert.ToInt32(cmd.ExecuteScalar())
        cmd.Dispose()
        connection.Close()
    Catch ex As Exception
        AuditTrail(vbLogEventTypeError, "IsConfigurationEnabled :Error opening SQL Connection ")
    End Try
    Return IsEnabled
End Function

I want to connect to database only once.
As the value in database never changes (or changes rarely).
Is there a way to achieve this?

Dale K
  • 25,246
  • 15
  • 42
  • 71
  • 2
    Then you will have to store this value somewhere, maybe in some singleton or static property? That may depend on the application type (in a web app, usually a static value isn't the right choice) – Hans Kesting Oct 13 '22 at 06:23
  • Don't worry about it too much. ADO.NET uses connection pooling so the cost of opening / closing the connection over and over is not too much of a problem see https://learn.microsoft.com/en-us/dotnet/framework/data/adonet/connection-pooling – SSS Oct 13 '22 at 06:25
  • 3
    `connect to db only once.` so don't call the method multiple times. Store the result in a field or property and reuse it. BTW this method is *extremely* buggy. It returns the wrong type (integer instead of boolean) In case of error it leaks the connection, maintains any locks on the table *and* returns a bogus value, `0`. Remove the `try/catch` block and use `Using` when declaring the connection to ensure it's closed even in case of errors. *Don't* hide the exception either. A missing database doesn't mean a disabled setting. It means the database is down – Panagiotis Kanavos Oct 13 '22 at 06:26
  • @PanagiotisKanavos , I will do this sir. I appreciate your excellent observational skills. – Karthik Karnam Oct 13 '22 at 06:37

1 Answers1

2

Storing the return value in the calling code is probably the best option. However, you can also use lazy initialization as follows.

Public Function IsConfigurationEnabled() As Integer
  Static isEnabled? As Integer
  If isEnabled.HasValue Then
    Return isEnabled.Value
  End If
  Dim sqlText As String = "select value from [dbo].[Settings] where Configuration='XXdX'"
  Using connection = New SqlConnection()
    connection.ConnectionString = "Data Source=localhost;Initial Catalog=Junk;Integrated Security=True"
    Try
      connection.Open()
      Using cmd = New SqlCommand(sqlText, connection)
        isEnabled = Convert.ToInt32(cmd.ExecuteScalar())
      End Using
      connection.Close()
    Catch ex As Exception
      AuditTrail(vbLogEventTypeError, "IsConfigurationEnabled :Error opening SQL Connection ")
    End Try
  End Using
  Return IsEnabled
End Function

It would be controversial to use the above code in a real project, but I hope it is instructive.

SSS
  • 4,807
  • 1
  • 23
  • 44
  • I liked this approach a lot :) Can I know the reasons Y this is controversial ? – Karthik Karnam Oct 13 '22 at 08:33
  • 1
    One problem is that the connection string is hardcoded, instead of gotten from a config – Hans Kesting Oct 13 '22 at 12:52
  • I think this approach is a bit controversial for the following reasons: 1. the static variable `isEnabled` stays around after the scope it is in is exited, 2. the method has a side-effect, and appears idempotent but isn't - for example, you might get unpredictable crashes that only happen when the routine is called the first time 3. you need to restart the program to reload the value - if the value in the database changes , calling this routine won't get the new value. That said, I used to looove this pattern :D – SSS Nov 03 '22 at 03:22