Is the database connection in this class "reusable"?

Question

I'm new to asp.net so this might be really basic question, but i cant figure it out.

I found a bit of code on the internet, that connects to database. And i created a namespace and some classes to use the same code in different projects.

The code and my class is the following:

namespace databaseFunctions
{
    public class databaseConnection
    {
private static string databaseConnectionString()
        {
            return "DRIVER={MySQL ODBC 5.1 Driver}; ........";
        }

        public static DataTable getFromDatabase(string SQL)
        {
            DataTable rt = new DataTable();
            DataSet ds = new DataSet();
            OdbcDataAdapter da = new OdbcDataAdapter();
            OdbcConnection con = new OdbcConnection(databaseConnectionString());
            OdbcCommand cmd = new OdbcCommand(SQL, con);
            da.SelectCommand = cmd;
            da.Fill(ds);
            try
            {
                rt = ds.Tables[0];
            }
            catch
            {   
                rt = null;
            }
            return rt;
        }

        public static Boolean insertIntoDatabase(string SQL)
        {

            OdbcDataAdapter da = new OdbcDataAdapter();
            OdbcConnection con = new OdbcConnection(databaseConnectionString());
            OdbcCommand cmd = new OdbcCommand(SQL, con);
            con.Open();
            try
            {
                cmd.ExecuteNonQuery();
                return true;
            }
            catch
            {
                return false;
            }

        }

}

There is no problem getting data from database, or insert data into some database. But. when i try to get the last_insert_id() from the mysql database. i only get a zero.

This is why i think that this piece of code I've created and copied from internet, creates a new connection for every time i call the "getFromDatabase(SQL)"

Is there anyone that could help me with fixing this class getFromDatabase() to keep the databaseconnection alive until i tell the program to abandon the connection?

I guess it is the "new OdbcConnection" that should be changed? Is it possible to check if there already is a connection alive? I've done this hundreds of times in classic asp, but now, with classes and stuff. I'm totally lost.

Answer 1

The problem you face is that you've coded yourself into a "new connection per action" corner. What you really want to aim for,and is considered best practice, is "new connection per batch of actions".

What I recommend in this case is to open connection when required, and close when disposed. What we'll do is move the odbc adapters to a larger scoped variable so that it can be accessed within the class.

namespace databaseFunctions
{
    public class databaseConnection:IDisposable
    {
        private OdbcConnection con;
        private string connectionString;

        public databaseConnection(string connectionString){
            this.connectionString = connectionString;
        }


        public void OpenConnection(){
            if (con == null || con.IsClosed ){ // we make sure we're only opening connection once.
                con = new OdbcConnection(this.connectionString);
            }
        }
        public void CloseConnection(){
            if (con != null && con.IsOpen){ // I'm making stuff up here
                con.Close();
            }
        }

        public DataTable getFromDatabase(string SQL)
        {
            OpenConnection();

            DataTable rt = new DataTable();
            DataSet ds = new DataSet();
            OdbcCommand cmd = new OdbcCommand(SQL, con);
            da.SelectCommand = cmd;
            da.Fill(ds);
            try
            {
                rt = ds.Tables[0];
            }
            catch
            {   
                rt = null;
            }
            return rt;
        }

        public Boolean insertIntoDatabase(string SQL)
        {
            OpenConnection();

            OdbcCommand cmd = new OdbcCommand(SQL, con);
            con.Open();
            try
            {
                cmd.ExecuteNonQuery();
                return true;
            }
            catch
            {
                return false;
            }

        }


        // Implementing IDisposable method
        public void Dispose(){
            CloseConenction();
        }
    }
}

Now the next time you use your class do something like

using (DatabaseConnection db = new DatabaseConnection()){
    db.InsertIntoDatabase(...);
    db.GetLastInsertID();
    db.GetFromDatabase(...);
}

At the end of that code block, because it is IDisposeable, it will close that connection for you in the dispose method.

---

Things I changed:

• implemented IDisposable interface
• changed methods from static to class methods.
• added new methods for opening closing connection
• moved connection variable to class level scope
• added an argument to the constructor that lets you pass in a connection string (you should put this connection string in you Web.Config

---

Edits:

• constructor takes in connectionString per suggestion.

---

Answer 2

Yes, the code you posted is creating a new database connection every time a method is called, but that's not a problem. The problem is that it is not disposing the connection properly. The way to handle something like this is as follows:

using (OdbcConnection con = new OdbcConnection("yourconnectionsstring"))
{
   con.open();
   OdbcCommand command = new OdbcCommand("command_text",con);
   command.ExecuteQuery(); //or what ever you need to do
}

That way the connection is being disposed properly since using is just syntactic sugar for try/finally

What you need to do is execute the 2 sql statements in the same transaction in a way that you insert the record in the first sql statement and retrieve the last inserted id on the next insert before ending the transaction. For example:

 using (OdbcConnection con = new OdbcConnection("yourconnectionsstring"))
    {
       con.open();
       OdbcTransaction tran = con.BeginTransaction()
       OdbcCommand command = new OdbcCommand("first_sql_statement_here",con);
       command.ExecuteNonQuery(); 
       command.CommandText = "select last_insert_id();";
       int result =command.ExecuteScalar(); 
       tran.commit();
    }

That is pretty much the idea.

Answer 3

You should let the connection pool handle your connections; That means you Close() every connection as soon as possible, and only create a new one at the last possible moment.

So yes - keep creating new ones for separate transactions.