1

Recently I found myself in C#, doing some SQL. This is not exactly my area of expertise. I wrote some code that looks pretty ugly, and am failing to find a better solution anyplace. Many of the answers here on SO pose SQL injection risks, and are essentially doing the same thing im doing in one way or another. The situation is, I have a form which a user provides a list of Store ID's in a form. When they click a button, a CSV export will be generated using the stores they provide as exclusion criteria for the query. The way I accomplished this is by setting up my SQL string as a constant, then using a string builder to dynamically append @X in an IN clause. The code looks fairly bad but heres a quick snippet to better explain. For example, say the SQL query is this

private readonly String _SELECT_UNEXPECTED_TOTES = "SELECT * FROM TABLE WHERE TABLE.Store IN ";

I then do the following (stores is an array of strings, sb is a string builder):

                //get enough room for all the stores
                var sqlParams = new SqlParameter[stores.Length];
                conn.Open();
                //append our query
                sb.Append(_SELECT_UNEXPORTED_TOTES);
                //open the IN list
                sb.Append("(");
                //build the in list
                int I = 0;
                foreach (String s in stores)
                {
                    sb.Append("@");
                    sb.Append(I);
                    sb.Append(",");
                    I++;
                }
                //trim the trailing ,
                sb.Length -= 1;
                sb.Append(")");
                //make the actual parameters
                I = 0;
                foreach (String s in stores)
                {
                    sqlParams[I] = new SqlParameter("@" + I, SqlDbType.VarChar);
                    sqlParams[I].Value = s;
                    I++;
                }

Later on in code I then use these SQL params in a SqlStatement object. Does .NET provide a better way to accomplish this? I dont know a lot about .NETs SQL objects, and for all I know this solution could be just as bad as a simple string replace... Any advice is welcome.

Mark W
  • 2,791
  • 1
  • 21
  • 44
  • 1
    I don't know an exact way for your situation, but according to your words, you can try EntityFramework. – İsmet Alkan Nov 04 '14 at 15:10
  • @IsThatSo The application im working on is pretty small. Its a quick job for a client, and I think, after watching the Entity Framework video, that Entity Framework is major overkill. I don't need to know much about the DB in this app. Its just, get a list of ID's, query, then spit out CSV. I was hoping for a strait forward solution that doesn't require elaborate API's. – Mark W Nov 04 '14 at 15:33
  • your solution is pretty straightforward. joining the strings, you can use `String.Join(", ", stores)` to make your code "better". – İsmet Alkan Nov 04 '14 at 15:37
  • String.Join would require a String.replace, which im against. If I try to use a single param (say @InList) and replace it with the String.join result, I would end up with an IN clause with a single value IN ( 'x, y, z') – Mark W Nov 04 '14 at 15:40
  • 1
    there are some great answers here: http://stackoverflow.com/questions/43249/t-sql-stored-procedure-that-accepts-multiple-id-values – İsmet Alkan Nov 04 '14 at 15:45
  • Use some variation on `SQLCommand` and either, pass a "Table Valued Parameter" or a parameter for each item in the `IN` list. The important point is that the call occurs via `sp_exectuteSql` and that the variable parts of the statement are parameters. – Jodrell Nov 04 '14 at 15:49
  • 1
    Your code may be ugly but it does avoid injection attacks. – Jodrell Nov 04 '14 at 15:54

2 Answers2

0

I don't believe there is an easier way of doing this through ADO.NET. But as some other users had mentioned Entity Framework would make this considerably cleaner and it really is not difficult to setup, especially on a small app. It would turn your above code into:

var data = context.TABLE.Where(t => stores.Contains(t.Store)).ToList();
mikej
  • 1
  • 1
0

Here's a much more LINQ-y way to accomplish this:

string[] storeIds = { "1Store", "2Store", "RedStore", "BlueStore" };
using (SqlCommand cmd = new SqlCommand())
{
    var paras = storeIds.Select((store, index) => new
        {
            name = String.Format("@p{0}", index),
            value = store
        });
    StringBuilder sb = new StringBuilder();
    sb.Append("select * from myTable");
    sb.Append(" where myTable.storeid in (");
    sb.Append(String.Join(",", paras.Select(p => p.name).ToList()));
    sb.Append(")");
    cmd.CommandText = sb.ToString();
    foreach (var para in paras)
    {
        cmd.Parameters.AddWithValue(para.name, para.value);
    }
    cmd.Connection = conn;
    // Now do whatever you want with the command ...

The "magic" being the use of the Enumerable.Select<TSource, TResult> Method (IEnumerable<TSource>, Func<Source, Int32, TResult>) overload, which gives you access to the index as well as the value of the elements in your array of IDs.

Edmund Schweppe
  • 4,992
  • 1
  • 20
  • 26