How to use SqlDataReader in a loop?

Question

I need to process the data from the list using SqlDataReader. To do this, I wrote a for loop in which the data for the query will be supplied and get into SqlDataReader. But after the first iteration, the loop breaks. An error is displayed that you need to close SqlDataReader.

List<string> days = new List<string>() { "Monday", "Tuesday", "Wednesday", "Thursday", "Friday" };

SqlConnection cnn = new SqlConnection(@"Con");
cnn.Open();

SqlCommand cmd = cnn.CreateCommand();
List<string> data = new List<string>();

for (int i = 0; i < days.Count;i++)
{
    cmd.CommandText = $@"select * from Table where Day = '{days[i]}'";
    
    SqlDataReader reader = cmd.ExecuteReader();
    
    while (reader.Read())
    {
        data.Add(reader[0].ToString());
    }
}

Here, for example, I use the "days" list, but in the program itself the list is obtained from a query, so it can be larger. This is where the error occurs. I tried to close SqlDataReader, but the problem is that it cannot be opened back. In any case, I need to somehow get data from SqlDataReader in a loop.

Always use parameterized sql and avoid string concatenation to add values to sql statements. This mitigates SQL Injection vulnerabilities and ensures values are passed to the statement correctly. See How can I add user-supplied input to an SQL statement?, Exploits of a Mom, and Why do we always prefer using parameters in SQL statements?. — Igor
– Igor, Commented Apr 2, 2023 at 13:31
Even when the parameter values are known it is a good habit to use parameters instead. — Igor
– Igor, Commented Apr 2, 2023 at 13:32
Implement a class whose properties map to the table columns and then try opening and closing your connection inside the loop — user16612111
– user16612111, Commented Apr 2, 2023 at 13:35
The main idea - query in a loop - is suspicious. SQL query is expencive: we should pass query via net (time), RDBM must parse it (time), optimize (time), validate credential, execute (and may be read from HDD), return results... If yo want to rob a bank, do it once and take a million, and stop keep doing it whenever you need a tenner for a glass of beer. Here we can easly change loop into a single in — Dmitrii Bychenko
– Dmitrii Bychenko, Commented Apr 2, 2023 at 13:41

Dmitrii Bychenko · Accepted Answer · 2023-04-02 13:32:28Z

Yes, you should Dispose instances that implement IDisposable: a quick patch, but not the best soultion is to add using:

...

for (int i = 0; i < days.Count; i++) {
  cmd.CommandText = $@"select * from Table where Day = '{days[i]}'";
    
  // using to Dispose reader
  using (SqlDataReader reader = cmd.ExecuteReader()) {
    while (reader.Read())
      data.Add(reader[0].ToString());
  }
}

A better aproach is to call query just once (SQL is expensive):

var data = new List<string>();

var days = new List<string>() { 
  "Monday", "Tuesday", "Wednesday", "Thursday", "Friday" 
};

using SqlConnection cnn = new SqlConnection(@"Con");

cnn.Open();

//TODO: change * into the only field(s) you want to fetch
// Since days are list of constants I haven't created bind variables:
//   - No sql injection
//   - Constants are specific and in specific order (workdays) so
//     I want the query be specific and not mixed with in (:param1, ... :paramN)
//     for arbitrary days 
string sql = 
  $@"select *
       from Table
      where day in ({string.Join(", ", days.Select(day => $"'{day}'"))})";

using SqlCommand cmd = cnn.CreateCommand(sql, cnn);

using SqlDataReader reader = cmd.ExecuteReader();

while (reader.Read()) {
  data.Add(Convert.ToString(reader[0]));
}

Collectives™ on Stack Overflow

How to use SqlDataReader in a loop?

1 Answer 1

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related