Search code examples
c#sqlasp.netsql-serverdatareader

Best way to structure multiple queries in one method c# asp.net


I have a page load method that loads asp dropdowns with sql queries to my SQL Server 2012 database. I'm new to this and basically independently learned a lot of needed to be done for the co op project I'm working on.

I've been running into problems with connections not being closed properly and having my connection pool blow up with only moderate usage of my app so I've been trying to improve how I'm executing my queries in my c# code behind. But I am not confident in my understanding of this so I'm going to post an example of my code and hope that someone much more fluent might be able to guide me a bit.

string constr = ConfigurationManager.ConnectionStrings["CurrencyDb"].ConnectionString;
                using (SqlConnection con = new SqlConnection(constr)) {
                    using (SqlCommand cmd = new SqlCommand("SELECT * FROM dbo.Category")) {
                        try {
                            cmd.CommandType = CommandType.Text;
                            cmd.Connection = con;
                            con.Open();

                            //Populate Category Dropdown
                            DDCategory.DataSource = cmd.ExecuteReader();
                            DDCategory.DataTextField = "CategoryName";
                            DDCategory.DataValueField = "CategoryId";
                            DDCategory.DataBind();
                        }
                        catch (SqlException sqlex) {
                            throw new Exception("SQL Exception loading data from database. " + sqlex.Message);
                        }

                        catch (Exception ex) {
                            throw new Exception("Error loading Category data from database. " + ex.Message);
                        }
                    }

                    using (SqlCommand cmd = new SqlCommand("SELECT * FROM dbo.SubCategory ORDER BY SubCategoryName")) {
                        try {
                            cmd.CommandType = CommandType.Text;
                            cmd.Connection = con;

                            //Populate SubCategory Dropdown
                            DDSubCategory.DataSource = cmd.ExecuteReader();
                            DDSubCategory.DataTextField = "SubCategoryName";
                            DDSubCategory.DataValueField = "SubCategoryId";
                            DDSubCategory.DataBind();
                        }
                        catch (SqlException sqlex) {
                            throw new Exception("SQL Exception loading data from database. " + sqlex.Message);
                        }

                        catch (Exception ex) {
                            throw new Exception("Error loading Subcategory data from database. " + ex.Message);
                        }
                    }
                }

The above are two queries of about 8 on my page load method.

My latest error was

There is already an open DataReader associated with this Command which must be closed first.

This prompted me to ask the question, I set MultipleActiveResultSets=true in my Web.config connection string and my app works now, but I feel as though that's a patch to cover what is likely crappy code.

What is the best practice for doing this? Thanks so much in advance!


Solution

  • To your immediate question, you are using two cmds per one connection. You could be better off using one-trip, one command, with a multiple-result.

    This will allow you to properly .Close() your datareader and your connection...."returning them back to the pool"

    Since you are using SqlServer, it supports multiple resultsets in one "trip".

    SELECT * FROM dbo.Category;SELECT * FROM dbo.SubCategory
    

    Use that as your select statement.

    Use an .ExecuteReader() method.

    And use a .NextResult() to move from one result (category), to the next one (subcategory)

    You can see a more complete sample here:

    https://msdn.microsoft.com/en-us/library/system.data.idatareader.nextresult(v=vs.110).aspx

    Or even with Entity Framework here:

    https://msdn.microsoft.com/en-us/library/jj691402%28v=vs.113%29.aspx?f=255&MSPPError=-2147217396

    Now some extra stuff.

    You are mixing your datalayer and your presentation layer.

    You should have your datalayer populate "Dto" sometimes referred to as "Poco" objects, and return those to the Presentation layer.

    public class Category
     public string CategoryKey{get;set;}
     public string CategoryName{get;set;}
    public ICollection<SubCategory> SubCategories {get;set;}
    

    ..

     public class SubCategory
     public string SubCategoryKey{get;set;}
     public string SubCategoryName{get;set;}
    

    ..

    public class MyDataLayerObject
        public ICollection<Category> GetAllCategories()
    {
        ICollection<Category> categories;
        ICollection<SubCategory> subcats;
    
        // write a datareader call here, and use it to populate multiple Category and SubCategory objects
    // make sure you close the datareader when done
    //now "match up" the subcats to its parent category
    }
    

    Then have your presentation layer "bind" to the ICollection of Categories.

    You might have a business layer between the presentation and datalayers, but at least to the presentation and datalayers.

    I also answered a question at the link below...that is similar to yours. Their objects are "Question" and "Answer" where a Question has 0:N number of Answers.

    Find my answer at the below question.

    Return objects with populated list properties from stored procedure

    Note, you can just use the 2 select queries in a string (the second line of my/this answer), aka, you don't have to create a stored procedure.

    Another option is to use nuget to get Microsoft.EnterpriseLibrary.Data. This encapsulates alot of great practices for you...including closing connections. You'll still have to close your datareader, but the code is alot cleaner.