SqlDataReader to read into List

Question

I am writing a method in C# to query a SQL Server Express database from a WCF service. I have to use ADO.NET to do this (then rewrite it with LINQ later on).

The method takes two strings (fname, lname) then returns a "Health Insurance NO" attribute from the matching record. I want to read this into a list (there are some other attribs to retrieve as well).

The current code returns an empty list. Where am I going wrong?

public List<string> GetPatientInfo(string fname, string lname)
{
    string connString = "Data Source=.\\SQLEXPRESS;AttachDbFilename=C:\\Users\\xxxx\\Documents\\Visual Studio 2010\\Projects\\ADOWebApp\\ADOWebApp\\App_Data\\ADODatabase.mdf;Integrated Security=True;User Instance=True";

    SqlConnection conn = new SqlConnection(connString);

    string sqlquery = "SELECT Patient.* FROM Patient WHERE ([First Name] = '"+fname+"') AND ([Last Name] = '"+lname+"')";
    SqlCommand command = new SqlCommand(sqlquery, conn);
    DataTable dt = new DataTable();

    List<string> result = new List<string>();

    using (conn)
    {
        conn.Open();

        using (SqlDataReader reader = command.ExecuteReader())
        {
            while (reader != null && reader.Read())
            {
               dt.Load(reader);
               result.Add(Convert.ToString(reader["Health Insurance NO"]));
            }
        }
     }

     return result;
}

Well debug it. Does it enter the `while` block? Does it actually execute the `result.Add` line? Also execute the same thing against the database yourself: take the value of `sqlquery` and run it on the database manually. — Arran, Oct 10 '13 at 15:29
@Arran, you're right... it's not executing that line at all. I did run the query manually to be sure it was correct, it returns the correct record. Apologies, I've been inserting breakpoints in and trying to debug this for almost 3 days now...! — reallybadatmath, Oct 10 '13 at 15:37
Knowing this now, I'm suspecting the connection string might be the culprit. I'll have a look and report back. — reallybadatmath, Oct 10 '13 at 15:39
Yep! It was the connection string. I'm going to delete this question as it's misleading for those that might stumble upon it. — reallybadatmath, Oct 10 '13 at 15:40
@reallybadatmath: You have the mentioned bugs anyway. So loading a `DataTable` in a loop with `Load` is pointless, using no sql-parameters is dangerous, using `SqldataReader.Read` with `DataTable.Load` is not necessary, using a `DataTable` at all when you need a list is redundant. — Tim Schmelter, Oct 10 '13 at 15:48
Thanks Tim. I did end up taking on all your suggestions with the code once I got it working. Many of the bugs came about from me trying all sorts of things to get the code working, my initial attempts did look similar to what you had provided. EDIT: I also thought I had already marked your code as the answer. My apologies. — reallybadatmath, Oct 11 '13 at 08:12

Tim Schmelter · Accepted Answer · 2013-10-10T15:36:05.677

You are trying to load a DataTable via DataTable.Load >in a loop<. You just need that once. You're also using reader.Read() in the loop. SqlDataReader.Read() advances the reader to the next record without to consume it. If you're going to use DataTable.Load you don't need to read the reader first. So you just have to remove the loop completely to load the table.

But since you want to return a list you don't need the DataTable at all, just loop the reader:

List<string> result = new List<string>();
using (conn)
{
    conn.Open();
    using (SqlDataReader reader = command.ExecuteReader())
    {
        while(reader.Read())
        {
            result.Add(Convert.ToString(reader["Health Insurance NO"]));
        }
    }
}

Apart from that, you are open for sql-injection without sql-parameters.

+1 Because I just needed a quick simple way to do this. HOWEVER - and this does not detract from this answer - you need to watch for cast errors or DBNull values. I only have 5 columns to read, so refactored into result.Add( createNewMyObjectFromReader() ) and handled each column in there.... if (System.DBNull.value != reader["someColumn"]) — Justin, Dec 21 '15 at 16:26

Jamie · Answer 2 · 2013-10-11T19:02:03.860

3

I would do it this way:

 conn.Open();
 using (SqlDataReader reader = command.ExecuteReader())
 {
     dt.Load(reader);                  
 }

 foreach (var row in dt.AsEnumerable())
 {
     result.Add(row["Health Insurance NO"].ToString());
 }

edited Oct 11 '13 at 19:02

answered Oct 10 '13 at 15:34

Jamie

3,094
1
18
28

5

You would load the table in a loop? ;-) – Tim Schmelter Oct 10 '13 at 15:37

SqlDataReader to read into List

2 Answers2

Linked