0

In my C# application, I am interfacing with a SQL Compact database. In this context, I have three tables; customer, list, and customerlist (customers and lists are a many-to-many relationship).

I have a function that I call when I want to delete a list. The function also deletes the related entries from the customerlist table too, just for cleanliness (since they are defunct if the list itself has been deleted).

My code is such:

    private void clearRedundantSubscriptions()
    {
        string sql;
        // Check if there are any entries in customerlist table which point to non-existing lists
        try
        {
            sql = "select distinct cl.listid from customerlist cl inner join list l on cl.listid != l.listid";
            SqlCeCommand cmdGetDisusedLists = new SqlCeCommand(sql, DbConnection.ceConnection);
            SqlCeDataReader reader = cmdGetDisusedLists.ExecuteReader();
            while (reader.Read())
            {
                DbFunctions.DeleteList(reader.GetInt32(0), false, false);
            }
        }
        catch (Exception ex)
        {
            MessageBox.Show("Error cleaning up list entries." + ex.Message);
        }
        return;
    }

    public static bool DeleteList(int id, bool display, bool close)
    {
        string sql;
        string title = "";
        bool ranOk = false;
        try
        {
            sql = "select ShortDesc from list where listid=" + id;
            DbFunctions.runSQL(sql, out title);
            sql = "delete from list where ListId=" + id;
            SqlCeCommand cmdDelList = new SqlCeCommand(sql, DbConnection.ceConnection);
            cmdDelList.ExecuteNonQuery();
            sql = "delete from customerlist where listid=" + id;
            SqlCeCommand cmdDelEntries = new SqlCeCommand(sql, DbConnection.ceConnection);
            cmdDelEntries.ExecuteNonQuery();
            if (display)
                General.doneWork(title + " list deleted.");
            ranOk = true;
        }
        catch (Exception ex)
        {
            if (display)
                MessageBox.Show("Unable to delete list. " + ex.Message);
        }
        finally
        {
            if (close)
                DbConnection.closeConnection();
        }
        return ranOk;
    }

    public static void closeConnection()
    {
        if (_sqlCeConnection != null)
            _sqlCeConnection.Close();
    }

You'll notice in my deletelist function, I pass in a bool parameter named 'close'. I added this because closing the connection to the database whilst inside a reader caused the above error. So now, if I want to use this function inside a reader, I call the deleteList function and make sure the 'close' parameter is passed in as false. Thats not a problem - this means that DbConnection.closeConnection is NOT called in this function.

So I am not closing the reader, nor the database connection. So any ideas why I am still getting this error?

Mike Baxter
  • 6,868
  • 17
  • 67
  • 115
  • You have forgotten to add the most important: _Where_ do you get the exception? – Tim Schmelter Mar 22 '13 at 11:15
  • On second pass of the reader :( – Mike Baxter Mar 22 '13 at 11:16
  • 1
    You are re-using the same connection for other sql commands. Read the list first, then DeleList() them. – H H Mar 22 '13 at 11:17
  • @TimSchmelter line 10. – Mike Baxter Mar 22 '13 at 11:20
  • @HenkHolterman could you give an example as an answer please? – Mike Baxter Mar 22 '13 at 11:23
  • In general: don't reuse the connection and always close it as soon as possible(best with `using`-statement). If connection-pooling is enabled(default) you'll achieve the opposite of what you're trying to do(slow and error-prone). http://stackoverflow.com/questions/9705637/executereader-requires-an-open-and-available-connection-the-connections-curren – Tim Schmelter Mar 22 '13 at 11:25
  • So I should be opening/closing connections where necessary, instead of re-using the same one? – Mike Baxter Mar 22 '13 at 11:42
  • @Teifi: Yes, actually you won't open/close the physical connection with pooling, you will just tell the pool that the connection can be reused when you close it. – Tim Schmelter Mar 22 '13 at 12:03

1 Answers1

1

I've modified your code, try this out.

I don't know what's happen inside your DbFunctions.runSQL method, so you might need to reopen the connection before that call.

private void clearRedundantSubscriptions()
{
    string sql;
    // Check if there are any entries in customerlist table which point to non-existing lists
    var list = new List<int>();
    try
    {
        if (DbConnection.ceConnection.State == ConnectionState.Closed)
            DbConnection.ceConnection.Open();

        sql = "select distinct cl.listid from customerlist cl inner join list l on cl.listid != l.listid";

        SqlCeCommand cmdGetDisusedLists = new SqlCeCommand(sql, DbConnection.ceConnection);
        SqlCeDataReader reader = cmdGetDisusedLists.ExecuteReader();
        while (reader.Read())
        {
            list.Add(reader.GetInt32(0));
        }
    }
    catch (Exception ex)
    {
        MessageBox.Show("Error cleaning up list entries." + ex.Message);
        throw;
    }
    finally
    {
        DbConnection.closeConnection();
    }

    foreach(var id in list)
    {
        DeleteList(id,false);

    }
    return;
}

public static bool DeleteList(int id, bool display)
{
    string sql;
    string title = "";
    bool ranOk = false;
    try
    {
        sql = "select ShortDesc from list where listid=" + id;
        DbFunctions.runSQL(sql, out title);


        sql = "delete from list where ListId=" + id;
        SqlCeCommand cmdDelList = new SqlCeCommand(sql, DbConnection.ceConnection);
        cmdDelList.ExecuteNonQuery();
        sql = "delete from customerlist where listid=" + id;
        SqlCeCommand cmdDelEntries = new SqlCeCommand(sql, DbConnection.ceConnection);
        cmdDelEntries.ExecuteNonQuery();
        if (display)
            General.doneWork(title + " list deleted.");
        ranOk = true;
    }
    catch (Exception ex)
    {
        if (display)
            MessageBox.Show("Unable to delete list. " + ex.Message);
    }
    finally
    {
            DbConnection.closeConnection();
    }
    return ranOk;
}

public static void closeConnection()
{
    if (_sqlCeConnection != null)
        _sqlCeConnection.Close();
}
codingadventures
  • 2,924
  • 2
  • 19
  • 36