skip to Main Content

I am taking over maintenance on some older ASP.NET projects.
I see code like the following all over the place.
Shouldn’t the database objects be wrapped in using statements?
The server is busy and runs for months as is without a reboot or any issues.

Also the GetDataTable()
Does a new DataTable();
and then returns a reference? To it.
How does this get cleaned up once the
DoSomething() finishes?

    private void DoSomething()
    {
        string Txt, sql;
        sql = "Select * from table;";
        DataTableReader dr = GetReader(sql);
        while (dr.Read())
        {
            Txt = dr["Txt"] + "";
        }
    }

    private DataTable GetDataTable(string sql)
    {
        SqlDataAdapter da = new SqlDataAdapter(sql, ConfigurationManager.AppSettings["DB"]);
        DataTable dt = new DataTable();
        da.Fill(dt);
        return dt;
    }

2

Answers


  1. Well, you did not post the GetReader function.

    However, the GetDataTable?

    Yes, all such routines should cleaned up. Thus, that code could be written as:

        public DataTable GetDataTable(string sql)
        {
            DataTable dt = new DataTable();
            using (SqlDataAdapter da = new SqlDataAdapter(sql,
                                       ConfigurationManager.AppSettings["DB"] ))
            {
                da.Fill(dt);
            }
            return dt;
        }
    

    So, yes, the correct approach is to ALWAYS wrap your connection, or adapter code with a connection in a using block. Thus, that will ensure everything is all cleaned up.

    As you note, the site been running for years without issue, and that’s just a great testimony as to how well .net cleans up itself, and runs anyway.

    .net has a built-in connection pool system, and it automatic recycles the connection pool for you. So, in theory you can write some less then ideal code, and everything continues to work rather well.

    In fact, before .net connection pools, then the developer had to write code to maintain active connections. Now, since they are pooled, then with a using block, the connections are still in the pool even when disposed in code, and thus the “old school” concepts of keeping a connection open all the time don’t apply anymore.

    However, such code should still have using blocks.

    Often you can get away say with a command object, or even a data adaptor object NOT wrapped in a using block, but if the connection object is used in that code or created? Then a using block should be used.

    In that second sample, a separate connection object not being created, and thus the using block should apply to the data adaptor.

    The first example is more problematic, since returning a reader means the table stays open. In that case, a dr dispose at the end of the code routine should be used.

    I thus never return a data reader, and would have returned a data table, of which the 2nd example code shows how (and then use a for/each on the datatable.rows in place of a while (dr.read).

    Login or Signup to reply.
  2. If something implements IDisposable, then you should make sure it is disposed of properly.

    Either by using a using block (if it is used within a method), or by implementing IDisposable yourself (if it is used at class level) so you can dispose of it when your class is Dispose()d

    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search