CODEWITHSUNDEEP
c#sql-server
Herrozerro
Herrozerro

Reputation: 1681

Returning the SqlDataReader

I am trying to either pass a reader by reference or to have one returned. And I am having issues with it on the return.

public static SqlDataReader GetSql(string businessUnit, string taskId)
{
            const string connstring = "Connection string";
            SqlConnection conn = new SqlConnection(connstring);
            SqlCommand command = new SqlCommand();
            SqlDataReader reader;
            try
            {
                conn.Open();
                command.Connection = conn;
                command.CommandType = CommandType.Text;
                command.CommandText =
                    "SELECT * FROM Audits WHERE BusinessUnit = @BU AND TaskID = @TID";
                command.Parameters.AddWithValue("@BU", businessUnit);
                command.Parameters.AddWithValue("@TID", taskId);
                return reader = command.ExecuteReader(CommandBehavior.CloseConnection);
            }
            catch (Exception ex)
            {
                return null;
            }
            finally
            {
                conn.Close();
            }
}


SqlDataReader reader = QaqcsqlLib.GetSql("job", "Task1");

if (reader.HasRows)
{
   while (reader.Read())
      MessageBox.Show(reader[0].ToString());
}

but I get the following error

Invalid attempt to call HasRows when reader is closed.

Any Ideas?

Upvotes: 2

Views: 2743

Answers (4)

Scott Chamberlain
Scott Chamberlain

Reputation: 127603

John is right about why you are having the problem but you don't need to pass in a connection.

I can see that you are already using the SqlCommand.ExecuteReader(CommandBehavior) overload when you start your reader and passing in CommandBehavior.CloseConnection. However the thing you are doing wrong is you are never disposing of the reader that is returned from your function. Remove the finally block then wrap your returned reader in a using block. This will close the underlying connection for you when you exit the block (becuse you passed in CommandBehavior.CloseConnection)

using(SqlDataReader reader = QaqcsqlLib.GetSql("job", "Task1"))
{
    while (reader != null && reader.Read()) //Added null check as your GetSql could return null.
      MessageBox.Show(reader[0].ToString());
}

I also stripped out reader.HasRows because it is unnessasary. If no results where returned the first call to reader.Read() will return false and the code inside the while loop will never execute.

You still need to close the connection when a exception occurs but you can just move the close from the finally in to the catch.

catch (Exception ex)
{
    conn.Dispose(); //Disposing closes the connection.
    return null;
}

Upvotes: 4

Ɖiamond ǤeezeƦ
Ɖiamond ǤeezeƦ

Reputation: 3331

Jon Skeet and Scott Chamberlain are both correct. If you wish to keep with your way of structuring the code (as opposed to Jon Skeet's approach), you will need to modify your code so that the connection is closed should an error occur while executing the SqlCommand.

public static SqlDataReader GetSql(string businessUnit, string taskId)
{
    const string connstring = "Connection string";
    SqlConnection conn = null;
    SqlCommand command = null;
    SqlDataReader reader = null;
    try
    {
        conn = new SqlConnection(connstring);
        command = new SqlCommand();
        command.Connection = conn;
        command.CommandType = CommandType.Text;
        command.CommandText = "SELECT * FROM Audits WHERE BusinessUnit = @BU AND TaskID = @TID";
        command.Parameters.AddWithValue("@BU", businessUnit);
        command.Parameters.AddWithValue("@TID", taskId);
        conn.Open();
        reader = command.ExecuteReader(CommandBehavior.CloseConnection);
        conn = null;
        return reader;
    }
    catch (Exception ex)
    {
        return null;
    }
    finally
    {
        if (conn != null) conn.Dispose();
        if (command != null) command.Dispose();
    }
}

Upvotes: 0

Jon Skeet
Jon Skeet

Reputation: 1504122

This is the problem:

finally
{
    conn.Close();
}

You're closing the connection before the method returns. The reader isn't going to be able to function without an open connection.

(It's not clear why you've got the reader variable at all, given that you only use it when returning.)

Returning a SqlDataReader in a method which itself opens the connection is generally tricky - because it means you don't have a nice way of closing the connection. Better would be to let the caller pass in the connection, at which point you can have:

using (var connection = new SqlConnection(...))
{
    using (var reader = QaqcsqlLib.GetSql(connection, "job", "Task1"))
    {
        // Use the reader here
    }
}

EDIT: As noted by Scott, your use of CommandBehavior.CloseConnection would allow closing the reader to close the connection. However, it makes other things trickier. For example, you'd have to dispose of the connection if an exception occurs (because then the caller won't have a chance), but not otherwise - I still prefer my approach.

Upvotes: 12

Rolando
Rolando

Reputation: 762

you can try follow the next example http://msdn.microsoft.com/en-us/library/haa3afyz.aspx .

You shouldn't call to close method before to use your reader.

Upvotes: 0

Related Questions