Closing MySql datareader connection

Question

So this is a little bit code-ceptionlike.

I have a function that is checking the last ID in a table, this function is called within another function. At the end of that function, I have another function that's opening another datareader.

Error:

There is already an open Datareader associated with this connection which must be closed first.

getLastIdfromDB()

public string getLastIdFromDB()
{
     int lastIndex;
     string lastID ="";
     var dbCon = DB_connect.Instance();

     if (dbCon.IsConnect())
     {
         MySqlCommand cmd2 = new MySqlCommand("SELECT ID FROM `competitor`", dbCon.Connection);

         try
         {
             MySqlDataReader reader = cmd2.ExecuteReader();

             while (reader.Read())
             {
                   string item = reader2["ID"].ToString();
                   lastIndex = int.Parse(item);
                   lastIndex++;
                   lastID = lastIndex.ToString();
              }
         }
         catch (Exception ex)
         {
              MessageBox.Show("Error:" + ex.Message);
         }
     }

     return lastID;
}

This function is later-on used in this function:

private void addPlayerBtn_Click(object sender, EventArgs e)
{
     ListViewItem lvi = new ListViewItem(getLastIdFromDB());
     .........................................^ 
     ...                                    HERE
     ...
     ...   irrelevant code removed
     .........................................

            var dbCon = DB_connect.Instance();

            if (dbCon.IsConnect())
            {
                MySqlCommand cmd = new MySqlCommand("INSERT INTO `competitor`(`ID`, `Name`, `Age`) VALUES(@idSql,@NameSql,@AgeSql)", dbCon.Connection);
                cmd.Parameters.AddWithValue("@idSql", getLastIdFromDB());
                cmd.Parameters.AddWithValue("@NameSql", playerName.Text);
                cmd.Parameters.AddWithValue("@AgeSql", playerAge.Text);

                try
                {
                    cmd.ExecuteNonQuery();
                    listView1.Items.Clear();
                }
                catch (Exception ex)
                {
                    MessageBox.Show("Error:" + ex.Message);
                    dbCon.Connection.Close();
                }
                finally 
                { 
                    updateListView();
                }
        }
}

What would be the best way for me to solve this problem and in the future be sure to close my connections properly?

UPDATE: (per request, included DB_connect)

    class DB_connect
    {
        private DB_connect()
        {
        }

        private string databaseName = "simhopp";

        public string DatabaseName
        {
            get { return databaseName; }
            set { databaseName = value; }
        }

        public string Password { get; set; }
        private MySqlConnection connection = null;

        public MySqlConnection Connection
        {
            get { return connection; }
        }

        private static DB_connect _instance = null;

        public static DB_connect Instance()
        {
            if (_instance == null)
                _instance = new DB_connect();
            return _instance;
        }

        public bool IsConnect()
        {
            bool result = true;

            try
            {
                if (Connection == null)
                {
                    if (String.IsNullOrEmpty(databaseName))
                        result = false;

                    string connstring = string.Format("Server=localhost; database={0}; UID=root;", databaseName);

                    connection = new MySqlConnection(connstring);
                    connection.Open();

                    result = true;
                }
            }
            catch (Exception ex)
            {
                Console.Write("Error: " + ex.Message);
            }

            return result;
        }

        public void Close()
        {
            connection.Close();
        }
    }
}

Answer 1

You are trying to have multiple open readers on the same connection. This is commonly called "MARS" (multiple active result sets). MySql seems to have no support for it.

You will have to either limit yourself to one open reader at a time, or use more than one connection, so you can have one connection for each reader.

My suggestion would be to throw away that singleton-like thingy and instead use connection pooling and proper using blocks.

Answer 2

As suggested by Pikoh in the comments, using the using clause indeed solved it for me.

Working code-snippet:

getLastIdFromDB

using (MySqlDataReader reader2 = cmd2.ExecuteReader()) { 

      while (reader2.Read())
      {
         string item = reader2["ID"].ToString();
         lastIndex = int.Parse(item);
         lastIndex++;
         lastID = lastIndex.ToString();
      }
}

Answer 3

Your connection handling here is not good. You need to ditch the DB_connect. No need to maintain a single connection - just open and close the connection each time you need it. Under the covers, ADO.NET will "pool" the connection for you, so that you don't actually have to wait to reconnect.

For any object that implements IDisposable you need to either call .Dispose() on it in a finally block, or wrap it in a using statement. That ensures your resources are properly disposed of. I recommend the using statement, because it helps keep the scope clear.

Your naming conventions should conform to C# standards. Methods that return a boolean should be like IsConnected, not IsConnect. addPlayerBtn_Click should be AddPlayerButton_Click. getLastIdFromDB should be GetlastIdFromDb or getLastIdFromDatabase.

public string GetLastIdFromDatabase()
{
     int lastIndex;
     string lastID ="";

     using (var connection = new MySqlConnection(Configuration.ConnectionString))
     using (var command = new MySqlCommand("query", connection))
     {
         connection.Open();
         MySqlDataReader reader = cmd2.ExecuteReader();

         while (reader.Read())
         {
               string item = reader2["ID"].ToString();
               lastIndex = int.Parse(item);
               lastIndex++;
               lastID = lastIndex.ToString();
         }   
     }

     return lastID;
}

Note, your query is bad too. I suspect you're using a string data type instead of a number, even though your ID's are number based. You should switch your column to a number data type, then select the max() number. Or use an autoincrementing column or sequence to get the next ID. Reading every single row to determine the next ID and incrementing a counter not good.