views:

384

answers:

3

Since I'm using Postgresql and can't use LINQ to SQL, I wrote my own wrapper classes.

This is a part of the Student class:

public class Student : User
{
    private static NpgsqlConnection connection = null;

    private const string TABLE_NAME = "students";

    public int Id { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string Password { get; set; }

    /// <summary>
    /// Reads data from the data reader and moves it to the Student class.
    /// </summary>
    private static void ReadFields(Student student, NpgsqlDataReader dr)
    {
        student.Id = Int32.Parse(dr["id"].ToString());
        student.FirstName = dr["first_name"].ToString();
        student.LastName = dr["last_name"].ToString();
        student.Password = dr["password"].ToString();
    }

    /// <summary>
    /// Updates the student
    /// </summary>
    public void Update()
    {
        Connect();
        Run(String.Format("UPDATE " + TABLE_NAME + " SET first_name='{0}', last_name='{1}', password='{2}' WHERE id={3}", FirstName, LastName, Password, Id));
        connection.Dispose();
    }

    /// <summary>
    /// Inserts a new student
    /// </summary>
    public void Insert()
    {
        Connect();
        Run(String.Format("INSERT INTO " + TABLE_NAME + " (first_name, last_name, password) VALUES ('{0}', '{1}', '{2}')",FirstName, LastName, Password));
        connection.Dispose();
    }

    private static void Run(string queryString)
    {
        NpgsqlCommand cmd = new NpgsqlCommand(queryString, connection);
        cmd.ExecuteScalar();
        cmd.Dispose();
    }

    private static void Connect()
    {
        connection = new NpgsqlConnection(String.Format("Server=localhost;Database=db;Uid=uid;Password=pass;pooling=false"));
        connection.Open();
    }

    //....

So as you see with every INSERT, DELETE, UPDATE request I'm using Connect() method which connects to the database. I didn't realize how stupid it was before I had to wait for 10 minutes to have 500 rows inserted, as there were 500 connections to the database.

So I decided to move Connection property to a static DB class.

public static class DB
{
    private static NpgsqlConnection connection = null;
    public static NpgsqlConnection Connection
    {
        get
        {
            if (connection == null)
            {
                connection = new NpgsqlConnection(String.Format("Server=localhost;Database=db;Uid=uid;Password=pass;pooling=false"));
                connection.Open();
            }
            return connection;
        }
    }

    public static void Run(string queryString)
    {
        NpgsqlCommand cmd = new NpgsqlCommand(queryString, connection);
        cmd.ExecuteScalar();
        cmd.Dispose();
    }
}

It works now! I replaces all Run methods in the Student class with DB.Run

But I want to know if it will work fine with a lot of people online, not me only. I'm not sure how static things work with ASP.NET, maybe it'll eat a lot of memory?..

+5  A: 

It is better not to store the connection in a static field. Create the connection object on demand and let the connection pooling manage your connections.

Gabriel McAdams
Why can storing connection in a static field be dangerous?
Alex
For one, it limits you to a single connection, where as connection pooling uses as many connections as you need, in the most effecient way possible (and you don't have to worry about it).
Gabriel McAdams
this link has some information that might be helpful: http://blog.jemm.net/articles/databases/best-practices-database-connections/
Gabriel McAdams
+3  A: 

You can enable connection pooling for PostgreSQL and let the pooler manage connections for you. Then you can use either piece of code without worry. Even when you issue multiple open/close commands the pooler will optimize them.

This provides you more flexibility and less worry about a custom management solution, also less code and edge cases. It will depend on the database provider you're using. Something in the connection string like:

Pooling: True or False. Controls whether connection pooling is used. Default = True


If you need a database provider that uses connection pooling for Postgres one option is npgsql: Npgsql is a .Net data provider for Postgresql. It supports connection pooling as described in the docs.

John K
Thank you! Pooling really helped. So now can I not worry about opening hundreds of connections and use the old way, or should I still do some optimization?
Alex
+1  A: 

Static classes are singletons. The danger here is what they reference. Because they always stay alive, everything they keep a reference to will not be garbage collected.

To see if this is the case, profile your web servers memory. If it always grows and never shrinks, you may be constantly adding references in a static class which never get collected.

In all honesty though, I'd create it only as needed, and completely avoid all of this.

EDIT:

I mean don't worry about sharing one single connection object across your data access layer. If the provider you're using supports connection pooling, then it will handle the actual connections made to the database. Just use and dispose of your connection objects as needed at any point in your data access layer.

using (var connection = new NpgsqlConnection("your connection string"))
{
    //your data access stuff.
}

I know code like this is rather big, bulky, and repetitive, but that's ADO.NET. As long as you isolate these calls in their own classes by creating a data access library/layer, it's very manageable. Hiding the ADO.NET objects in a static class is dangerous, because you'll inevitably forget to close a connection or call Dispose() somewhere. Plus you run the risk of building a large object graph that will never get garbage collected.

Aaron Daniels
Okay, that's what I thought. I already had a bad experience with static classes before... What do you mean by 'avoid all of this'? Avoid the wrappers and access SQL directly?
Alex