0

I want to turn the results of a SQL query into a C# list. I've tried using the following code which doesn't work - Visual Studio returns

An unhandled exception of type 'System.InvalidCastException' occurred in System.Data.dll

Code:

public List<string> carList(int id)
{
        List<string> list = new List<string>();

        SqlConnection conn = new SqlConnection(connStr);
        conn.Open();

        string query = "SELECT ID FROM Cars WHERE custID = " + id;
        SqlCommand cmd = new SqlCommand(query, conn);

        using (SqlDataReader reader = cmd.ExecuteReader())
        {
            while (reader.Read())
            {
                list.Add(reader.GetString(0));
            }
        }

        conn.Close();

        return list;
    }

The result of the SQL query should just be a single column with one or more ID's (integers). I then want to put these all into a list. Hopefully this gives you a bit of an idea of what I'm trying to do.

If someone could point out what I'm doing wrong here then I'd be very thankful.

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Viva
  • 45
  • 2
  • 2
  • 8
  • 1
    "doesn't work" - how? Do you get an error? The list is empty? Something else? – stuartd Jan 20 '17 at 16:43
  • @stuartd Pardon me, should have included that. Visual Studio returns: "An unhandled exception of type 'System.InvalidCastException' occurred in System.Data.dll" – Viva Jan 20 '17 at 16:45
  • 1
    You should get the value as integer instead of GetString(0) and then call .ToString() on the value to add it in the list. – Ali Baig Jan 20 '17 at 16:48
  • 2
    What's the data type of Cars.ID? I bet it's not a `string`. Try `reader.GetInt32(0).ToString()` – Dour High Arch Jan 20 '17 at 16:48
  • @AliBaig that actually did the trick... damn I feel stupid but thank you! – Viva Jan 20 '17 at 16:52
  • @DourHighArch It indeed wasn't, tried that exact code and it's all working now. Thanks! – Viva Jan 20 '17 at 16:54
  • 2
    [SQL Injection alert](http://msdn.microsoft.com/en-us/library/ms161953%28v=sql.105%29.aspx) - you should **not** concatenate together your SQL statements - use **parametrized queries** instead to avoid SQL injection – marc_s Jan 20 '17 at 17:06
  • @marc_s - normally I'm with you, however it's being called within a function that accepts the id as an int (and the value isn't being altered at all after this)...if something else was to be passed through, the function call would fail. That being said, it's poor practice to query like this, and it should be parameterized for consistency sake. – user2366842 Jan 20 '17 at 17:12
  • 1
    @marc_s Ok, I didn't realise this could pose a security risk, thanks for pointing out. – Viva Jan 20 '17 at 17:16
  • Maybe not in this instance (unless I missed something with my above statement?) but this certainly isn't best practice, and had it not been wrapped in a function it most certainly is a security risk. – user2366842 Jan 20 '17 at 17:20
  • @user2366842 As far as I can tell you're right but as you both said this isn't the best practice and any way I can improve my code is welcome. So thanks! – Viva Jan 20 '17 at 17:30
  • If `Cars.ID` is an integer why are you turning it into a string? – Dour High Arch Jan 20 '17 at 18:49

2 Answers2

1

Give it a try as suggested in the comments

while (reader.Read())
{
    list.Add(reader.GetInt32(0).ToString());
}

The type of data on 0th index should be an integer which should be converted to a string before adding to the list.

Ali Baig
  • 3,819
  • 4
  • 34
  • 47
1

If you want a more generic way to convert any single-column result set to a list of strings you can use:

while (reader.Read())
{
    list.Add(reader.GetValue(0).ToString());
}
D Stanley
  • 149,601
  • 11
  • 178
  • 240