3

I have written following code. I have opened the database connection for once for one query I want to execute another query. I have written the code below. But i think there is a mistake Can anyone help me please?

public void check()
{
    try
    {
        OdbcConnection myOdbcConnection = new OdbcConnection(con1);
        OdbcCommand myOdbcCommand = myOdbcConnection.CreateCommand();
        String sSQL = "SELECT * FROM(select tdate from tbl_IThelpdesk order by call_no desc)where ROWNUM = 1"; //last record of the call_no column
        myOdbcCommand.CommandText = sSQL;
        myOdbcConnection.Open();
        OdbcDataReader myOdbcDataReader = myOdbcCommand.ExecuteReader();
        if (!myOdbcDataReader.Read())
        {
            txtDate.Text = DateTime.Now.ToShortDateString();
            string strcallno = DateTime.Now.Year.ToString("d2") + DateTime.Now.Month.ToString("d2") + DateTime.Now.Day.ToString("d2");
            txtcall.Text = "ITHD" + strcallno + "001";
            myOdbcConnection.Close();
            myOdbcDataReader.Close();
        }
        else
        {
            DateTime today = DateTime.Parse(DateTime.Now.ToShortDateString());
            if (myOdbcDataReader[0].ToString() == today.ToString())
            {
                myOdbcConnection.Close();
                myOdbcDataReader.Close();
                myOdbcConnection.Open();
                OdbcCommand myOdbcCommand1 = myOdbcConnection.CreateCommand();
                String SQLmax = "SELECT max(call_no) FROM TBL_IThelpdesk";
                myOdbcCommand1.CommandText = SQLmax;

                OdbcDataReader myOdbcDataReader1 = myOdbcCommand1.ExecuteReader();
                while (myOdbcDataReader1.Read() != false)
                {
                    txtcall.Text = myOdbcDataReader1[0].ToString().Trim();
                }
                myOdbcDataReader1.Close();
                myOdbcDataReader.Close();
                myOdbcConnection.Close(); 
            }
        }
    }
    catch (Exception e)
    {
        lblEmpty.Text = e.Message;
        lblEmpty.Visible = true;
    }
}
svick
  • 236,525
  • 50
  • 385
  • 514
Shanna
  • 753
  • 4
  • 14
  • 34
  • 1
    Hello and Welcome to Programmers. Questions such as these are off-topic here as they are explicitly about implementation. They are, however, on topic on Stack Overflow. We'll see about a migrate shortly. Have a pleasant day. –  Feb 19 '13 at 04:44
  • Why do you think there is a mistake? Are you getting an error? Are you getting back the wrong data? Are you experiencing slowdowns? What is your question, precisely? – Avner Shahar-Kashtan Feb 19 '13 at 06:49

2 Answers2

11

Since database connections use a pool, you don't have to maintain the same connection for multiple queries; instead, open a connection when you need it, and close it as soon as possible to free up the resources.

See: http://msdn.microsoft.com/en-us/library/8xx3tyca(v=vs.80).aspx
See also: C# SQLConnection pooling

Note that you've not used using() { } pattern. Given that OdbcConnection and similar types implement IDisposable, you should embed them into a using in order for them to be disposed without waiting the garbage collector.

See: http://msdn.microsoft.com/en-us/library/yh598w02.aspx

Community
  • 1
  • 1
Arseni Mourzenko
  • 50,338
  • 35
  • 112
  • 199
2

You're hitting the database twice when you only need to hit it once. You're getting the latest date then going to the database again to get the corresponding call_no. This is unsafe as the max(call_no) could change even in the small amount of time between the 2 queries.

//get the latest [call_no] and [tdate]. No need for a 2cd trip with max(call_no)
SELECT * FROM(select call_no, tdate from tbl_IThelpdesk order by call_no desc)where ROWNUM = 1

Also the data access code is mixed with UI code. You should create data access methods that do 1 thing; return the data you want. This will make it much easier to follow the main flow of your algorithm.

mike
  • 329
  • 2
  • 11