1
\$\begingroup\$

My goal is to create a class that I can call from my webpages like this. Similar to a data access layer?

string sql = "SELECT EMPLOYEE_ID, FIRST, LAST, HIRE_DATE " + 
             "FROM EMPLOYEES ";
DataTable dtEmployees = SqlConnectionHelpers.TimeManagementDataTable(sql);

The class itself looks like this and is working so far (it's in the AppCode, folder on my website), but my boss has me questioning whether it will work in all cases or with a bunch of users accessing the webpages.

public static class SqlConnectionHelpers
{
    private static SqlConnection timeManagementCon= new SqlConnection("Data Source=XXX.XXX.XXX.XXX\\TM;Initial Catalog=TimeManagement;User ID=payroll;Password=*****");

    #region TimeManagement
    public static DataTable TimeManagementDataTable(string sql)
    {
        DataTable dt = new DataTable();
        SqlDataAdapter adapter = new SqlDataAdapter(sql, SqlConTimeManagement());
        adapter.Fill(dt);
        return dt;
    }

    public static string TimeManagementCommand(string sql)
    {
        SqlCommand command = new SqlCommand(sql, SqlConTimeManagement());

        var result = command.ExecuteScalar();

        if (result != null)
            return result.ToString();
        else
            return "Success";
    }

    private static SqlConnection SqlConTimeManagement()
    {
        if (timeManagementCon.State == ConnectionState.Open)
            return timeManagementCon;
        else
            timeManagementCon.Open();
        return timeManagementCon;
    }
    #endregion
}

So my questions are

  1. Is this thread safe? This is a word that comes up a lot when I was looking into this problem. Say 10 people have this open in 10 different tabs, are they all accessing the same static page? If so does this cause issues with multiple people trying to grab the same connection concurrently?

  2. Does this cost a lot of needless overhead, when I could just code it every time I needed it?

  3. Is this bad practice? If so how can I fix it?

\$\endgroup\$
  • 1
    \$\begingroup\$ I suggest not having the static variable for SqlConnection, instead obtain a new SqlConnection each time you need it ( use "Using" ) to make sure it's closed/disposed. \$\endgroup\$ – George Barwood Feb 14 at 6:17
  • \$\begingroup\$ You can remove static keyword from class and method. And create an object of the class and call method of that class. Also as per @GeorgeBarwood use using keyword so C# will take care of dispose of every object of the class. \$\endgroup\$ – Kevin Shah Apr 30 at 6:50
  • \$\begingroup\$ And one more thing instead of creating a specific method try to create a generic method like instead of TimeManagementDataTable you can change it name to generic GetDataTable to use the same method in every page \$\endgroup\$ – Kevin Shah Apr 30 at 6:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.