Is it safe to use static methods for accessing database in following scenario

Question

I am upgrading a asp website to asp.net. I am trying to follow multi teir approach. My basic dal layer is as follows which returns a datatable and insert a given query.

using System;
using System.Configuration;
using System.Data;
using MySql.Data.MySqlClient;

public class mydatautility
{
    public mydatautility()
    {
    }
    public static DataTable Table(string query)
    {
        string constr = ConfigurationManager.ConnectionStrings["db_con"].ConnectionString;
        DataTable table = new DataTable();
        try
        {
            using (MySqlConnection con = new MySqlConnection(constr))
            {
                con.Close();
                MySqlCommand com = new MySqlCommand(query, con);
                MySqlDataAdapter da = new MySqlDataAdapter(com);
                con.Open();
                da.Fill(table);
                con.Close();
                da = null;
                com = null;
                con.Dispose();
            }
        }
        catch (Exception)
        {
        }
        return table;
    }
    public static int Insert_intoemployee(string query)
    {
        string constr = ConfigurationManager.ConnectionStrings["db_con"].ConnectionString;
        int done = 0;
        try
        {
            using (MySqlConnection con = new MySqlConnection(constr))
            {
                MySqlCommand com = new MySqlCommand(query, con);
                con.Open();
                done = com.ExecuteNonQuery();
                con.Close();
                com = null;
                con.Dispose();
            }
        }
        catch (Exception)
        {
        }
        return done;
    }
}

I am not sure what will happen when 2 concurrent queries are run.
How can I test it for concurrency problem?

I feel I should point out that `ExecuteNonQuery` returns the number of rows affected. So your `Insert` method does not actually return the primary key.. if that was what you were intending. You're looking for `ExecuteScalar`. — Simon Whitehead, Mar 05 '13 at 05:05
No its alright to get no. of rows affected.I dont need primary key. — Ratna, Mar 05 '13 at 05:06

score 2 · Answer 1 · edited May 23 '17 at 11:46

2

Using static methods in this scenario is safe. The variables inside the static method is isolated from concurrent calls! see this link too: variable in static methods inside static class

edited May 23 '17 at 11:46

Community

1
1

answered Mar 05 '13 at 05:32

Nishanth Nair

2,975
2
19
22

Sunny · Accepted Answer · 2013-03-05T05:45:41.710

There will no concurrency problem as each request has its own thread and static methods have individual call stacks for each thread. However, there are some suggestions in code.

using System;
using System.Configuration;
using System.Data;
using MySql.Data.MySqlClient;

public static class mydatautility//change to Utilities
{
    public mydatautility()//not required in this scenario
    {
    }
    public static DataTable Table(string query) //change method name to GetTable
    {
        string constr = ConfigurationManager.ConnectionStrings["db_con"].ConnectionString;
        DataTable table = new DataTable();
        try
        {
            using (MySqlConnection con = new MySqlConnection(constr))
            {
                con.Close();//not required
                using(MySqlCommand com = new MySqlCommand(query, con))
                {
                MySqlDataAdapter da = new MySqlDataAdapter(com);
                con.Open();
                da.Fill(table);
                con.Close();
                da = null;// reduntant, not required
                com = null;// reduntant, not required
                con.Dispose();// reduntant, not required
                }
            }
        }
        catch (Exception)
        {
        }
        return table;
    }
    public static bool InsertEmployee(string query)// consider changing int to bool since you only require result of operation
    {
        string constr = ConfigurationManager.ConnectionStrings["db_con"].ConnectionString;
        int done = 0;
        try
        {
            using (MySqlConnection con = new MySqlConnection(constr))
            {
                Using(MySqlCommand com = new MySqlCommand(query, con))
                {
                con.Open();
                done = com.ExecuteNonQuery();
                con.Close();
                com = null;// reduntant, not required
                con.Dispose();// reduntant, not required
                }
            }
        }
        catch (Exception)
        {
        }
        return done > 0; // checks rows affected greater than 0
    }
}

+0. You have nice code suggestions (also missed `con.Close()` before end of `using` block), but you explanation why there is no concurency problems is questionable. The fact that every request runs on its own thread only increase chance of problems, and in no way eliminate them. — Alexei Levenkov, Mar 05 '13 at 05:48
I also mentioned in this case, static methods have individual call stacks per thread, so there will be no concurrency problems. And I read that each request executes on separate thread, http://stackoverflow.com/questions/1416351/asp-net-does-every-httprequest-get-its-own-thread, http://stackoverflow.com/questions/8084143/does-an-asp-net-http-request-translate-to-1-thread — Sunny, Mar 05 '13 at 05:57
but then why there is concurrency problem when a variable is declared in a page class. i have seen the variable holds the same variable for all instance of pages, for which i have to shift to session. Correct me if i am wrong. — Ratna, Mar 05 '13 at 06:36
In page, you are associating static variables with partial class. So, every request has same copy. Here in this case also, by declaring a static variable inside Utilities class, will be single copy for all threads. If you change it, all references get updated. These static variables are stored per AppDomain. — Sunny, Mar 05 '13 at 06:42
In these methods, you are passing variables, which are non-static and expires (deleted from call stack) as soon as function ends. So, each function call has its own call stack. Hope this helps. — Sunny, Mar 05 '13 at 06:46

score 2 · Answer 3 · answered Jun 23 '15 at 09:55

2

I think it is safe, but bad practice. If you use static methods to access live resource, then how do you want to unit test them? You can not really mock the database access any more.

answered Jun 23 '15 at 09:55

HamoriZ

2,370
18
38

Is it safe to use static methods for accessing database in following scenario

3 Answers3