Parameterized search query in C#

Question

In the UI I have a single search text box into which users can type a query. The query will split the string and compare each word to each configured column in the database and return the matches.

The following code works, but I don't know if there is a better way that I'm missing.

/// <summary>
/// Returns a list of users, either active, inactive or all
/// and can perform a search for specific users
/// </summary>
/// <param name="status">1 = active 2 = inactive 3 = all</param>
/// <param name="query">string to use for search or blank = no search</param>
/// <returns>List of Users</returns>
public static List<UM_UserData> GetUsers(int status, string query)
{
    List<UM_UserData> data = new List<UM_UserData>();

    using (MySqlConnection con = new MySqlConnection(Cfg.connectionString))
    {
        con.Open();

        string sql = GetUsersQuery(status, query);

        using (MySqlCommand cmd = new MySqlCommand(sql, con))
        {
            if (status != 3)
            {
                cmd.Parameters.AddWithValue("@status", status);
            }

            if(query != "")
            {
                string[] words = query.Split(' ');
                int x = 1;
                foreach(string word in words)
                {
                    string param = "@param" + x;
                    cmd.Parameters.AddWithValue(param, "%" + word + "%");
                    x++;
                }
            }

            using (MySqlDataReader reader = cmd.ExecuteReader())
            {
                while(reader.Read())
                {
                    data.Add(new UM_UserData(reader));
                }
            }
        }
    }

    return data;
}

private static string GetUsersQuery(int status, string query)
{
    string sql = "SELECT * FROM rw_db.v_sys_users";

    if (status != 3 || query != "")
    {
         sql += " WHERE 1 = 1 ";
    }

    if(status != 3)
    {
        sql += " AND `user_status` = @status ";
    }

    if(query == "") { return sql; }

    string[] words = query.Split(' ');
    string[] fields = new string[] { "user_empid", "user_firstname", "user_lastname", "user_username" };

    sql += buildSearch(words, fields);

    return sql;
}

private static string buildSearch(string[] words, string[] fields)
{
    string s = " AND((";

    int x = 1;

    foreach(string word in words)
    {
        if(x != 1) { s += "AND("; }
        string paramName = " @param" + x +" ";

        int y = 1;

        foreach(string field in fields)
        {
            if(y != 1) { s += " OR "; }

            s += field + " LIKE " + paramName + " ";
            y++;
        }

        s += " ) ";
        x++;
    }

    s += ")";

    return s;
}

---

Thank you everyone, I appreciate all of yall's advice.

Here is what I ended up with.

---

public static List<UM_UserData> GetUsers(int status, string search)
{
    List<UM_UserData> data = new List<UM_UserData>();
    UM_GetUserQuery sqlObj = new UM_GetUserQuery(status, search);

    using (MySqlConnection con = new MySqlConnection(Cfg.connectionString))
    {
        con.Open();
        using (MySqlCommand cmd = new MySqlCommand(sqlObj.sqlQuery, con))
        {
            cmd.Parameters.AddRange(sqlObj.parameters.ToArray());

            using (MySqlDataReader reader = cmd.ExecuteReader())
            {
                while (reader.Read())
                {
                    data.Add(new UM_UserData(reader));
                }
            }
        }
    }

    return data;
}

internal class UM_GetUserQuery
{
    #region FIELDS
    public string sqlQuery { get; private set; }

    public List<MySqlParameter> parameters { get; private set; }
    #endregion

    #region CONSTRUCTORS
    public UM_GetUserQuery(int status, string search)
    {
        parameters = new List<MySqlParameter>();
        BuildQuery(status, search);
    }
    #endregion

    #region PRIVATE METHODS
    private void BuildQuery(int status, string search)
    {
        var builder = new StringBuilder("SELECT * FROM rw_db.v_sys_users");

        var condition = new StringBuilder();

        // IF STATUS != 3 "All Users"
        if(status != 3)
        {
            // ADD CONDITION FOR STATUS AND ADD PARAMETER TO COLLECTION
            condition.Append(" AND `user_status` = @statusParam ");
            parameters.Add(new MySqlParameter("@statusParam", status));
        }

        // IF SEARCH IS NOT EMPTY 
        if(!string.IsNullOrWhiteSpace(search))
        {
            string[] words = search.Split(' '); // SPLIT SEARCH USING SINGLE SPACE
            string[] fields = new string[] { "user_empid", "user_firstname", "user_lastname", "user_username" };

            // CREATE A NEW SEARCH OBJECT
            SearchObj searchObj = new SearchObj(words, fields);

            // APPEND SEARCH QUERY TO CONDITION
            condition.Append(searchObj.sql);
            // APPEND SEARCH PARAMETERS TO COLLECTION
            parameters.AddRange(searchObj.parameters);
        }

        // IF CONDITION IS NOT EMPTY
        if (condition.Length > 0)
        {
            // ADD CONDITION TO BUILDER
            builder.Append(" WHERE 1=1 ");
            builder.Append(condition);
        }

        // SET BUILDER TO SQL STRING
        sqlQuery = builder.ToString();
    }
    #endregion  
}

internal class SearchObj
{
    #region FIELDS
    public string sql { get; private set; }

    public List<MySqlParameter> parameters { get; private set; }
    #endregion

    #region CONSTRUCTORS
    public SearchObj(string[] words, string[] fields, bool useAND = true)
    {
        parameters = new List<MySqlParameter>();

        BuildSearch(words, fields, true);
    }
    #endregion

    #region PRIVATE METHODS
    private void BuildSearch(string[] words, string[] fields, bool useAND = true)
    {
        StringBuilder builder = new StringBuilder();

        string AndOR = useAND ? " AND(" : " OR(";

        builder.Append(" AND((");

        int x = 1;

        // LOOP THROUGH EACH SEARCH WORD IN WORDS
        foreach (string word in words)
        {
            // IF NOT FIRST WORD APPEND AND
            if (x != 1) { builder.Append(AndOR); }

            string paramName = string.Format("@param{0}", x);

            parameters.Add(new MySqlParameter(paramName, "%" + word + "%"));

            int y = 1;
            // LOOP THROUGH EACH FIELD
            foreach (string field in fields)
            {
                // IF NOT FIRST FIELD APPEND OR
                if (y != 1) { builder.Append(" OR "); }

                builder.Append($" {field} LIKE {paramName} ");

                y++;
            }

            builder.Append(")");
            x++;
        }

        builder.Append(")");

        sql = builder.ToString();
    }
    #endregion
}

Answer 1

In contrast to radarbob I find a StringBuilder very useful in this context.

Your code produces something like this:

SELECT * FROM rw_db.v_sys_users WHERE 1 = 1 AND user_status = @status AND((user_empid LIKE @param1 OR user_firstname LIKE @param1 OR user_lastname LIKE @param1 OR user_username LIKE @param1 ) AND(user_empid LIKE @param2 OR user_firstname LIKE @param2 OR user_lastname LIKE @param2 OR user_username LIKE @param2 ) AND(user_empid LIKE @param3 OR user_firstname LIKE @param3 OR user_lastname LIKE @param3 OR user_username LIKE @param3 ) )

which looks ok to me (except for 1 = 1 :-), but are you sure you want AND between the OR-sequences? I would say OR was a better choise.

Below find my rainy sunday afternoon version with some inline comments:

      class UserSearch
      {
        public List<UM_UserData> GetUsers(int status, string searchWords)
        {
          string connectionString = "<TODO: Connection String>";
          var parameters = GetParameters(searchWords);
          var sql = BuildSql(status, parameters);

          Console.WriteLine(sql);

          // TODO: Change SqlConnection etc. to MySqlCon... 
          // No need to fire up the connection until everything is ready...
          using (SqlConnection connection = new SqlConnection(connectionString))
          {
            connection.Open();

            using (SqlCommand command = new SqlCommand(sql, connection))
            {
              command.Parameters.AddRange(parameters.ToArray());
              var reader = command.ExecuteReader();
              List<UM_UserData> result = new List<UM_UserData>();

              while (reader.Read())
              {
                result.Add(new UM_UserData(reader));
              }

              return result;
            }
          }
        }

        List<SqlParameter> GetParameters(string searchWords)
        {
          if (string.IsNullOrWhiteSpace(searchWords))
          {
             return new List<SqlParameter>();
          }

          return searchWords
            .Split(' ')
            .Distinct()
            .Where(word => !string.IsNullOrWhiteSpace(word))
            .Select(word => word.Trim())
            .Select((word, i) => new SqlParameter($"@Param_{i}", $"%{word}%")) // TODO: change to a MySqlParameter
            .ToList();
        }

        string BuildSql(int status, List<SqlParameter> parameters)
        {
          // TODO: check for syntactical differences between MS Sql and MySql

          // Use a StringBuilder to build the strings, it is much more efficient and the code is easier to maintain and understand.
          var builder = new StringBuilder("SELECT * FROM rw_db.v_sys_users");

          if (parameters.Count > 0 || status != 3)
          {
            builder.Append(" WHERE ");

            if (parameters.Count > 0)
            {
              JoinFieldsAndParameters(builder, parameters);
            }

            // Add the status criteria as the last. It is easier to append it if necessary
            if (status != 3)
            {
              string andString = parameters.Count > 0 ? "AND" : "";
              string statusParam = $"@statusParam";
              builder.Append($" {andString} user_status = {statusParam} ");
              parameters.Add(new SqlParameter(statusParam, status));
            }
          }

          return builder.ToString();
        }

        void JoinFieldsAndParameters(StringBuilder builder, List<SqlParameter> parameters)
        {
          if (parameters.Count == 0)
            return;

          string[] fields = new string[] { "user_empid", "user_firstname", "user_lastname", "user_username" };


          foreach (var parameter in parameters)
          {
            builder.Append("(");

            foreach (var field in fields)
            {
              builder.Append($"{field} LIKE {parameter} OR ");
            }
            builder.Length -= 4; // Removes the last " OR "
            builder.Append(") AND "); // Are you sure you want AND here and not OR?
          }

          builder.Length -= 5; // Removes the last " AND "
        }
      }

Answer 2

use string.Format() it is easier to read and far less error prone

s += string.Format("{0} LIKE {1} ", field, paramName);

string paramName = string.Format(" @param{0} ", x);

---

 string[] fields = new string[] { "user_empid", "user_firstname", "user_lastname", "user_username" };

 sql += buildSearch(words, fields)

Why pass fields as a parameter if it is constant. Put it in BuildSearch() instead.

---

Capitalize method names - C# idiom.

---

Be precise: don't say "blank string" when you mean string.Empty (a.k.a. "empty string").

This: string.Empty not: "" Eliminates uncertainty due to eyesight, font style and size, accidental spacebar, etc.

Strings are reference types and may be null. Always use String.IsNullOrWhiteSpace() - preferred over string.IsNullOrEmpty. Either one definitely preferred over myString == ""

Test string parameters for null. Calling a method on a null string reference throws an exception of course. And to simplify downstream code I like to set null strings to string.Empty

public void SomeMethod( string stringParm ){
    stringParm = stringParm ?? string.Empty;  // null coalescence operator
    // ...
}

---

In BuildSearch():

 int x = 1;

foreach(string word in words)
{
    if(x != 1) { s += "AND("; }
    string paramName = " @param" + x +" ";

Return early if words is null/empty:

if (words == null || words.Length == 0) return; 
...

 int x = 1;
 s += "AND("; 
foreach(string word in words)
{
    string paramName = string.Format( " @param{0} ", x );

fields is constant, never null/empty.

y seems to never be used

So, this:

s += " OR ";

foreach(string field in fields)
{
    s += string.Format( "{0} LIKE {1} ", field, paramName );
 }

---

This does not make sense:

private static string GetUsersQuery(int status, string query)
{
   string sql = "SELECT * FROM rw_db.v_sys_users";

If this method is getting the query, why are you passing in the query?

If you are passing in the query why is a query being defined?

---

Proposed Experiment

The code structure is lousy. There is testing for status != 3 and query != null all over the place, and even repeated in a nested loop. I have to read and study a lot to figure out how the query is being built differently based on these conditionals.

Re-write the code to examine the conditionals up front and separate code for each path. This will result in some redundancy I guess but the point is to see how the code becomes clean and understandable.

Personally I'd go for clarity over some minor duplication in this case. But you should be able to DRY that up. Broader scoped common variables or via method parameters.

---

P.S.

Some readers may point out that I do not mention StringBuilder. I'm not feeling the need here. I think the proposed re-write will be far more helpful lesson in the long run.

Answer 3

You probably don't want to search for empty strings via equating:

query != ""

Using

!String.IsNullOrEmpty(query)

or

Length.query > 0

Would result in much better code performance.