23

I am having Invalid attempt to call Read when reader is closed error when I am doing 3 tier project in C# language. What I am trying to do is retrieve address data column by joining two tables together and display in a drop down list. Here is my data access layer:

public List<Distribution> getDistributionAll()
{
    List<Distribution> distributionAll = new List<Distribution>();
    string address;
    SqlDataReader dr = FoodBankDB.executeReader("SELECT b.addressLineOne FROM dbo.Beneficiaries b INNER JOIN dbo.Distributions d ON d.beneficiary = b.id");

    while (dr.Read())
    {
        address = dr["addressLineOne"].ToString();
        distributionAll.Add(new Distribution(address));
    }

    return distributionAll;
}

And this is my FoodBankDB class:

public class FoodBankDB
{
    public static string connectionString = Properties.Settings.Default.connectionString;
    public static SqlDataReader executeReader(string query)
    {
        SqlDataReader result = null;
        System.Diagnostics.Debug.WriteLine("FoodBankDB executeReader: " + query);
        SqlConnection connection = new SqlConnection(connectionString);
        SqlCommand command = new SqlCommand(query, connection);
        connection.Open();
        result = command.ExecuteReader();
        connection.Close();
        return result;
    }
}

I separated the these into two class so that whenever my connection string is changed, I can amend the whole project easily by changing the FoodBankDB class.

And this is my business logic layer:

public List<Distribution> getAllScheduledDistribution()
{
    List<Distribution> allDistribution = new List<Distribution>();
    Distribution distributionDAL = new Distribution();
    allDistribution = distributionDAL.getDistributionAll();
    return allDistribution;
}

And last but not least, my presentation layer:

List<Distribution> scheduledList = new List<Distribution>();
scheduledList = packBLL.getAllScheduledDistribution();
ddlScheduleList.DataSource = scheduledList;
ddlScheduleList.DataTextField = "address";
ddlScheduleList.DataValueField = "address";
ddlScheduleList.DataBind();

It was working well if I didn't split the data access layer and connection string class. Does anybody know how to solve this error?

Thanks in advance.

Updated portion

public static string GetConnectionString()
{
    return connectionString;
}
Jacob Shanley
  • 893
  • 1
  • 8
  • 19

2 Answers2

35

It doesn't work because you close the connection before returning the reader. Reader works only when the connection is open:

result = command.ExecuteReader();
connection.Close();

return result; // here the reader is not valid

Generally speaking, you should not be returning a reader to a business layer. Reader should be used only in the data access layer. It should be used and then it and the connection should be closed.

You should rather return an object that can work after the connection is closed, e.g. a DataSet or DataTable or alternatively a collection of DTO's. For example:

public List<Distribution> getDistributionAll()
{
    List<Distribution> distributionAll = new List<Distribution>();

    using (var connection = new SqlConnection(FoodBankDB.GetConnectionString())) // get your connection string from the other class here
    {
        SqlCommand command = new SqlCommand("SELECT b.addressLineOne FROM dbo.Beneficiaries b INNER JOIN dbo.Distributions d ON d.beneficiary = b.id", connection);
        connection.Open();
        using (var dr = command.ExecuteReader())
        {
            while (dr.Read())
            {
                string address = dr["addressLineOne"].ToString();

                distributionAll.Add(new Distribution(address));
            }
        }
    }

    return distributionAll;
}
Szymon
  • 42,577
  • 16
  • 96
  • 114
  • 1
    Would you mind to show me some example on how to solve this? Because I am kind of confused –  Dec 09 '13 at 12:04
  • 1
    See the example above. I hope there's no small mistakes as I cannot run the code now. – Szymon Dec 09 '13 at 12:08
  • Am I supposed to code the GetConnectionString() as updated portion? Or am I doing in the wrong way? –  Dec 09 '13 at 12:16
  • Yes, as of above code, you need the GetConnectionString() coded in. OR, you can use FoodBankDB.connectionString directly: using (var connection = new SqlConnection(FoodBankDB.connectionString)) .... – Bora Dec 09 '13 at 12:29
  • Yes, that's right. I did it this way as you said you wanted to have it separately. – Szymon Dec 09 '13 at 12:29
9

Previous one is a good example ... But you can also accomplish it by below code which is automatically close a connection instance when datareader.close() method called ...

reader = Sqlcmd.ExecuteReader(CommandBehavior.CloseConnection); 
Moumit
  • 8,314
  • 9
  • 55
  • 59
  • Sorry for posting on a very old post, but I want to know is it absolutely necessary that you need to call `datareader.close()` to close the connection? Wont it be automatically handled after initializing the instance with `CommandBehavior.CloseConnection` – hiFI Jan 30 '20 at 07:52
  • @hiFI .. no it will not. As datareader work's on open connection .. and there can be multiple result set .. so you need to explicitly close datareader ... the benefit of above parameter is it will call the related connection also to close – Moumit Jan 30 '20 at 15:43
  • Whatever said, your solution was a savior! It was easier than passing the connection as an `out` parameter. Thank You – hiFI Jan 31 '20 at 06:10