When running this as a loop, should I put the Open() and Close() outside or inside the loop?

Question

I have a piece of code

        await this._Conn.OpenAsync();
        using (SqlCommand cmd = new SqlCommand("AddOrUpdateAnswer", this._Conn))
        {
            cmd.CommandType = CommandType.StoredProcedure;
            cmd.Parameters.AddWithValue("@AnswerVal", Answer.AnswerVal);
            cmd.Parameters.AddWithValue("@QuestionId", Answer.QuestionId);
            cmd.Parameters.AddWithValue("@PartnerId", Answer.PartnerId);
            await cmd.ExecuteNonQueryAsync();
        }
        this._Conn.Close();

which is currently not inside a loop but now I want to run in inside a loop. My question is whether I should write it like

        for ( var Answer in Answers )
        {
            await this._Conn.OpenAsync();
            using (SqlCommand cmd = new SqlCommand("AddOrUpdateAnswer", this._Conn))
            {
                cmd.CommandType = CommandType.StoredProcedure;
                cmd.Parameters.AddWithValue("@AnswerVal", Answer.AnswerVal);
                cmd.Parameters.AddWithValue("@QuestionId", Answer.QuestionId);
                cmd.Parameters.AddWithValue("@PartnerId", Answer.PartnerId);
                await cmd.ExecuteNonQueryAsync();
            }
            this._Conn.Close();
        }

or can I have it opened for the entirety of the loop like

            await this._Conn.OpenAsync();
            for ( var Answer in Answers )
            {
                using (SqlCommand cmd = new SqlCommand("AddOrUpdateAnswer", this._Conn))
                {
                    cmd.CommandType = CommandType.StoredProcedure;
                    cmd.Parameters.AddWithValue("@AnswerVal", Answer.AnswerVal);
                    cmd.Parameters.AddWithValue("@QuestionId", Answer.QuestionId);
                    cmd.Parameters.AddWithValue("@PartnerId", Answer.PartnerId);
                    await cmd.ExecuteNonQueryAsync();
                }
            }
            this._Conn.Close();

Why or why not?

score 5 · Answer 1 · edited Mar 28 '19 at 07:16

5

There is some level of overhead opening a new connection, although connection pools do significantly reduce that overhead.

It is generally best to open the connection once, before your loop.

Your current code will never close the connection if an exception is thrown. You should use a using statement when opening the connection and not explicitly close it. Let the IDisposable pattern take care of that for you.

edited Mar 28 '19 at 07:16

Kols

3,641
2
34
42

answered Mar 11 '16 at 19:16

Eric J.

147,927
63
340
553

Can you show me what that `using` statement for the connection opening would look like?? – Subpar Web Dev Mar 11 '16 at 19:22
Have a look at http://stackoverflow.com/questions/16566547/do-using-statements-and-await-keywords-play-nicely-in-c-sharp – Eric J. Mar 11 '16 at 19:25

score 2 · Answer 2 · answered Mar 11 '16 at 19:34

Opening and closing the connection for each iteration is a wasted repetition:

for ( var Answer in Answers )
    {
        await this._Conn.OpenAsync();
        using (SqlCommand cmd = new SqlCommand("AddOrUpdateAnswer", this._Conn))
        {
            cmd.CommandType = CommandType.StoredProcedure;
            cmd.Parameters.AddWithValue("@AnswerVal", Answer.AnswerVal);
            cmd.Parameters.AddWithValue("@QuestionId", Answer.QuestionId);
            cmd.Parameters.AddWithValue("@PartnerId", Answer.PartnerId);
            await cmd.ExecuteNonQueryAsync();
        }
        this._Conn.Close();
    }

If you think about any database query that you make, you open the connection:

await this._Conn.OpenAsync();
// Do what is needed while the connection is open.
// Then close the connection.
this._Conn.Close();

Try not to confuse the issue with the fact that what you are doing while the connection is open happens to be a loop of what you did singularly while the connection was open.

It's like instead of fetching one apple while in the shop, you're fetching a basket of apples. There's no need to keep leaving the shop each time you fetch one apple and then going back in for the next apple.

When running this as a loop, should I put the Open() and Close() outside or inside the loop?

2 Answers2