4
\$\begingroup\$

I have a List of Results. My goal is to extract the Result property Method from all elements of List<Result>, make extracted elements unique and prepare a string for SQL statements like WHERE method IN ('A','B','C').

Here is the Result class and a List<Result> example. The solution is in the ListTransformation() method.

Result.cs

public class Result
{
    public int Id { get; set; }
    public string Method { get; set; }
}

Program.cs

A method value can be one method ("A") or more methods separated by a colon ("A:B:C").

static void ListTransformationTest()
{
    const string separator = ":";

    List<Result> results = new List<Result>
                { new Result { Id = 1, Method = null },
                  new Result { Id = 1, Method = "" },
                  new Result { Id = 2, Method = "A:B" },
                  new Result { Id = 3, Method = "B:C" },
                  new Result { Id = 4, Method = "A:C:B" }
                };

    string methods = string.Join(separator,
                                 results.Where(x => x.Method?.Length > 0)
                                        .Select(r => r.Method).ToArray());
    var uniqueMethods = methods.Split(separator[0]).Distinct();
    string uniqueMethodsSqlFormat =
            string.Join(",", uniqueMethods.Select(m => string.Format($"'{m}'")).ToArray());
}

I guess there are a lot of operations. Do you have any suggestions on improving the code or any alternative code?

\$\endgroup\$
0

2 Answers 2

3
\$\begingroup\$

Creating query (alternative)

Creating SQL like that is vulnerable to SQL Injection. You should use parameters instead.

Here's an example:

using (var cmd = new SqlCommand())
{
    var methods = results
        .SelectMany(x => x.Method?.Split(
            new[] { ':' }, 
            StringSplitOptions.RemoveEmptyEntries) ?? new string[0]
        ).Distinct();

    var methodParameterNames = new List<string>();
    foreach (var method in methods)
    {
        methodParameterNames.Add($"@Method{methodParameterNames.Count + 1}");
        cmd.Parameters.Add($"@Method{methodParameterNames.Count}", SqlDbType.NVarChar).Value = result;
    }
    var methodParameterString = string.Join(", ", methodParameterNames);

    // build the rest of the query
}

Extracting methods

To extract all methods use the SelectMany extension with the coalescing operator ?? so that you don't have to check for null later. You use an empty collection. The loop simply won't execute. Finally you get unique methods with Distinct.

Creating SQLParameters

In order to create a query for each result you create a SqlCommand. You add each method to a list with parameter names and at the same time add the parameters and the values to the command. The parameters have counters. The first counter needs to be +1 because it is just going to be added and it has to match the counter in the parameter line. At the end you build a string with parameter names that you can use to build the final query.

Example methodParameterString:

 @Method1, @Method2, @Method3

The query to create:

 var sql = $".. WHERE [Method] IN ({methodParameterString}) ..";

Review

string.Join(",", [...].ToArray());

There's no need to .ToArray() the collection for string.Join because it expects an IEnumerable<T>. Calling ToArray you loop over the collection twice.


const string separator = ":";
separator[0]

You can make the separator a char and drop the [0] which is very confusing.


.Where(x => x.Method?.Length > 0)

It's easier to do:

.Where(x => !string.IsNullOrEmpty(x.Method))
\$\endgroup\$
3
  • \$\begingroup\$ I knew that somebody can have better solution. Thank you! Just one note about SQL injection. In my case the SQL injection seems to me unlikely because methods are not becoming from user input. That is why I decided to do it simple. And method list will be part of SELECT statement (no update, delete, insert). Anyway no doubt that use of SQL parameters is preferred. \$\endgroup\$
    – Tomas Paul
    Dec 18, 2016 at 9:09
  • \$\begingroup\$ @TomášPaul ok ;-) I imagine it might sometimes be fine without parameters but I needed to mention the SQL Injection to not get downvoted. But on the other hand SQL Injection is mostly about a SELECT that is modified, see How can i update a table using SQL Injection?. \$\endgroup\$
    – t3chb0t
    Dec 18, 2016 at 9:54
  • \$\begingroup\$ @t13chb0t, yeah, right. DROP table joke is funny :-). I know that database hack. Not my case, no user input. I mentioned SELECT for my case because I suppose the select statement doesn't cause any harm to database records if it is not properly constructed. \$\endgroup\$
    – Tomas Paul
    Dec 18, 2016 at 11:44
1
\$\begingroup\$

You asked about another approach
If you get Methods into a collection then it gives you a base for a lot of stuff
This does not deal with the SQL part
I feel like you should separate the two

static void ListTransformationTest()
{
    ResultsB resultsB = new ResultsB();
    resultsB.Add(new ResultB(1, "a:b"));
    resultsB.Add(new ResultB(2, "c:b"));
    Debug.WriteLine(resultsB.MethodsString);
}

public class ResultB
{
    public int Id { get; set; }
    // if you need null then would need string?
    public HashSet<string> Methods { get; } = new HashSet<string>();
    public string MethodsString
    {
        get { return String.Join(", ", Methods); }
    } 
    public ResultB(int id)
    {
        Id = id;
    }
    public ResultB(int id, string methods)
    {
        char[] separators = { ':' };
        Id = id;
        if (!string.IsNullOrEmpty(methods))
        {
            foreach(string s in methods.Split(separators))
            {
                // this would throw out empty but you do that in the net 
                if(!string.IsNullOrEmpty(s))
                {
                    Methods.Add(s.Trim());
                }
            }
        }
    }
}
public class ResultsB : List<ResultB>
{
    public string MethodsString
    {
        get
        {
            return string.Join(", ", this.MethodsAll);
        }
    }
    public HashSet<string> MethodsAll
    {
        get
        {
            HashSet<string> hs = new HashSet<string>();
            foreach (ResultB resultB in this)
            {
                foreach (string s in resultB.Methods)
                {
                    hs.Add("'" + s + "'");
                }
            }
            return hs;
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.