C# Database static or not

Question

I am developing a tool which gets some information from a MSSQL database. There is an own database user for the access. I know how static works an when I should use them but in this case I am not sure.

I have an own Database class:

    class Database
    {
        private const String connString = "the connection string";
        private const String query1 = "";

        public Database()
        {
            // some initialization
        }

        // some different methods which calls CreateCommand and a queryString and
        // returns the result

        private static void CreateCommand(string queryString, string connectionString)
        {
            using (SqlConnection sqlConnection = new SqlConnection(connectionString))
            {
                sqlConnection.Open();
                using (SqlCommand command = new SqlCommand(queryString, sqlConnection)) 
                {
                    using (SqlDataReader dataReader = command.ExecuteReader()) 
                    {
                        if (dataReader != null)
                        {
                            while (dataReader.Read())
                            {

                            }
                        }
                    }
                }
            }
        }
    }

There are just select queries where I get data from the server. A own thread will call some database methods for example every hour an displays the result. Should I use a static class and static methods for this or a normal class with only the createCommand method static or nothing static?

Kind regards

Do you unit test your project? If yes, don't use static methods. They make it hard to test. — Sriram Sakthivel, Sep 17 '14 at 12:24
With this approach you are opening, executing and closing on EVERY SQL statement. This isn't very economical, especially if you might run 10 queries each time your one hour task kicks off. — Belogix, Sep 17 '14 at 12:25
Is this for educational purpose? If not, I suggest you use one of the many existing ORMs: [Entity Framework](http://msdn.microsoft.com/nl-be/data/ef.aspx), [NHibernate](http://nhforge.org/), [Dapper](https://github.com/StackExchange/dapper-dot-net),... — user247702, Sep 17 '14 at 12:26
@Belogix Then what you would suggest? Are you aware of connection pooling? — Sriram Sakthivel, Sep 17 '14 at 12:27
@Belogix, this is not true. But default ADO.NET will pool connections with the same connection string, holding and reusing an existing connection instead of setting up and tearing down for each query. http://msdn.microsoft.com/en-us/library/8xx3tyca(v=vs.110).aspx — Mike Chamberlain, Sep 17 '14 at 12:29
@MikeChamberlain and Sriram Sakthivel - What I am saying is that is not EXPLICIT. Your code should read as you intend it to work. By doing this you are relying on something under the hood protecting your performance. That is never a good idea! You should design it with you in control, not some safety net. It is good that it exists but what I was trying to highlight (without getting into detail) is the OP Needs to THINK about what they are actually doing with the database / connections / logic. Alternatives? Unit of Work for a starter! — Belogix, Sep 17 '14 at 12:32
I do not use unit tests for this project because it is a very small project. I know that i will open, execute and close in every query but on msdn and other stack overflow questions they recommended to use it in such a form. How can I use it better? — user2571670, Sep 17 '14 at 12:34
@user2571670, I think Belogix is misleading you, as one of the design decisions of the SQL Server ADO.NET driver was to *implicitly* provide pooling by default. Understanding this is fundamental to its use, and for someone to complain that code taking advantage of the feature is not explicit enough suggests that they do not understand the library deeply enough. The documentation for the `Close()` method even *explicitly* says that it merely returns the connection to the pool. Looking at it another way, to "explicitly" pool connections you are going to have to write your own pooling library. — Mike Chamberlain, Sep 17 '14 at 12:55
@MikeChamberlain - Not at all, you are missing the point... Let me put it simply... How would you (using the above code) implement `SqlTransaction` over multiple commands? — Belogix, Sep 17 '14 at 13:15
It just make no sense to me that you would pass in a connection string and query string but not pass anything out. How would several methods use this? You have no mechanism to direct the output. Why is a method passing the connection string? — paparazzo, Sep 17 '14 at 13:26
Ok thanks, I make it not static because I totally agree with your points. @Belogix before I found the using solution my idea was to create the database class where I create an instance of sqlconnection in the constructor and then I make methods for the statements and one method for opening the connection an one for closing. With this I had to create a instance of the database class in my thread class then open the connection, make the different queries and then close. I think with that SqlTransactions over multiple commands are possible? Just a question I don't need transactions in my project. — user2571670, Sep 17 '14 at 13:28

score 2 · Accepted Answer · edited May 23 '17 at 12:07

Except in specific cases, static methods should be avoided as they reduce testability and cause your design to become tightly coupled. A full discussion is outside the scope of this site, but here are a couple of places to start:

http://objcsharp.wordpress.com/2013/07/08/why-static-code-is-bad/ http://misko.hevery.com/2008/12/15/static-methods-are-death-to-testability/

In your case, however, your static method is private, which as pointed out by BG100, doesn't seem to make sense in the context of your instantiated class. Why not just declare it an instance (non-static) method, and keep your code flexible by passing in the connection string in the constructor:

class Database
{
    private string connString;

    public Database(string connString)
    {
        this.connString = connString;
    }

    private void CreateCommand(string queryString)
    {  
        using (SqlConnection sqlConnection = new SqlConnection(this.connString))
        {
            // etc.
        }
    }
}

But now you have the problem that your CreateCommand is returning void. Once you read the data from your reader, how are you returning it to the calling method? Perhaps you should take a look at using a DataSet instead:

https://stackoverflow.com/a/4099694/289319

Thanks good answer, I now know how to use it. Sorry for the wrong code I just test it without the using and the static method. I create the new code and asked because if it is better to use static or not in this case and I only use the code that you nearly know what I mean. I always try to make good code also if the project is very very small like in this case =) — user2571670, Sep 17 '14 at 13:46

score 1 · Answer 2 · answered Sep 17 '14 at 12:26

The simpliest way I can think of to describe static methods, is to say that if you declare your CreateCommand method as static, then it will prevent you from accessing the member variables (connString and query1) from within that method... so as you clearly need to access these, the you can't declare the method as static.

So the answer is no, don't declare it as static because you method will then no longer work.

_{There are also a few other tweaks that probably should be addressed as well, as mentioned in some of the comments, but this is beyond the scope of this question.}

Oh yes your right thats my fault, there you can see I am not so often use or need static — user2571670, Sep 17 '14 at 12:44

C# Database static or not

2 Answers2