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
– Simon Whitehead, Commented Mar 5, 2013 at 5:05
No its alright to get no. of rows affected.I dont need primary key. — Ratna
– Ratna, Commented Mar 5, 2013 at 5:06

Sunny · Accepted Answer · 2013-03-05 05:45:41Z

2

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
    }
}

edited Mar 5, 2013 at 5:45

answered Mar 5, 2013 at 5:36

Sunny

4,8195 gold badges39 silver badges76 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Alexei Levenkov Over a year ago

+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.

Sunny Over a year ago

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, stackoverflow.com/questions/1416351/…, stackoverflow.com/questions/8084143/…

Ratna Over a year ago

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.

Sunny Over a year ago

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 Over a year ago

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.

Community · Accepted Answer · 2017-05-23 11:46:04Z

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, 2017 at 11:46

CommunityBot

11 silver badge

answered Mar 5, 2013 at 5:32

Nishanth Nair

2,9652 gold badges22 silver badges22 bronze badges

Comments

HamoriZ · Accepted Answer · 2015-06-23 09:55:42Z

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, 2015 at 9:55

HamoriZ

2,45821 silver badges42 bronze badges

Collectives™ on Stack Overflow

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

3 Answers 3

5 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

5 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related