Passing parameters to a query

Question

I have a function that takes a Dictionary<String, Object> as an argument. It uses this Dictionary to create parameters for a query string.

private static TableAdapters dataSource = new TableAdapters();
private void findScriptsQueryButton_Click(object sender, EventArgs e)
{
  string fillBy = findByComboBox.SelectedItem.ToString();
  string server = serverComboBox.SelectedItem.ToString();
  Dictionary<String, Object> parameters = new Dictionary<String, Object>()
  {
    {"findBy", findByTextBox.Text}
  };
  dataGridView.DataSource = dataSource.GetDataSource(fillBy, server, parameters).Tables[0];
}

public class TableAdapters
{
  public DataSet GetDataSource(string fillBy, string server, Dictionary<String, Object> parameters)
  {
    string query = GetQuery[fillBy];
    SqlConnection connection = new SqlConnection();
    connection.ConnectionString = GetSqlConnection[server];
    SqlDataAdapter adapter = new SqlDataAdapter(query, connection);
    foreach (KeyValuePair<String, Object> p in parameters)
    {
      adapter.SelectCommand.Parameters.AddWithValue("@" + p.Key, (p.Value.ToString().Length == 0) ? DBNull.Value : p.Value);
    }
    DataSet dataSet = new DataSet();
    dataSet.Tables.Add(new DataTable());
    adapter.Fill(dataSet.Tables[0]);
    return dataSet;
  }
}

My primary problem with my code is:

1. I have to create a new dictionary of parameters every time before I call the function.
2. The parameter names are hard coded into both the dictionaries and the query strings.

The number of parameters for a query string in this particular program can be 1-11. Is there a better way for me to pass parameters to this function where I don't have to hard code the names? Mind you I don't want to go off parameter number either, because then I've become dependent on ensuring I pass the parameters to the function in the right order.

For brevity here is an example of one of the queries from the query dictionary that this particular instance of the function calls:

private static readonly Dictionary<String, String> GetQuery = new Dictionary<String, String>()
{
  {"Target VDN", 
   "SELECT * FROM table_name WHERE column_name = ISNULL(@findBy, column_name) "
  }, 
  // ...
}

In this case I am only dealing with one parameter but as I mentioned some of these queries in the dictionary can take up to 11 parameters.

Answer 1

You can take advantage of C#'s variable parameter arguments (keyword params). Like this -

public DataSet GetDataSource(string fillBy, string server, params KeyValuePair<String, Object>[] parameters)
    {
        string query = GetQuery[fillBy];
        SqlConnection connection = new SqlConnection();
        connection.ConnectionString = GetSqlConnection[server];
        SqlDataAdapter adapter = new SqlDataAdapter(query, connection);
        foreach (KeyValuePair<String, Object> p in parameters)
        {
            adapter.SelectCommand.Parameters.AddWithValue("@" + p.Key, (p.Value.ToString().Length == 0) ? DBNull.Value : p.Value);
        }
        DataSet dataSet = new DataSet();
        dataSet.Tables.Add(new DataTable());
        adapter.Fill(dataSet.Tables[0]);
        return dataSet;
    }

params is the keyword to use for taking variable numbers of arguments. Then you can just call it with variable arguments like -

   dataGridView.DataSource = dataSource.GetDataSource(fillBy, server, 
        new KeyValuePair<string, object>("findBy",findByTextBox.Text) 
        ).Tables[0];

and in another time you can call same function with -

   dataGridView.DataSource = dataSource.GetDataSource(fillBy, server, 
            new KeyValuePair<string, object>("findBy",findByTextBox.Text), 
            new KeyValuePair<string, object>("findBy2",findByTextBox2.Text)
        ).Tables[0];

This will atleast save you from defining a dictionary all the time.

Answer 2

The code isn't bad, I think you just need to properly wrap it and expose a better API. For instance, GetDataSource can stay, but it should be private and have a better-named and typed function that calls into it. Something like:

DataTable FindBy(String findBy)

You could also move the SQL string literal inside this function, having that GetQuery dictionary only helps if you need to access them from more than one place. With the SQL inside, it's right next to where you make the dictionary so the parameter names will be easier to keep in sync.

Answer 3

Regarding the Dictionary<> parameters: Do you need to be able to test if a member is already there? It doesn't seem like it.

Do you need to be able to pull out the value of a specific field? It doesn't seem like it.

The only thing you're using parameters for is to be enumerated later.

So you don't need a Dictionary, you just need any IEnumerable<KeyValuePair<String,String>>. Even an array would do. (I recommend the second member of the tuple should just be a String, because it's only used as a string; there's no reason to put objects into there.)

I really think you should look into LINQ especially LINQ-to-SQL. It allows a lot more flexibility and abstractness in building the query, it's just as easy to bind to a view, you don't have to do any string building, and parameterization is a lot easier. (I shudder to see that code where you concatenate a literal '@' to user input)

Also your current code has a bug that you treat all zero-length strings as DBNull, and it's suboptimal to create a new SQL connection for each query. LINQ takes care of that too. You can even have a dictionary of LINQ queries ready to be filled in, so you don't have to story any strings at all.

Answer 4

The parameter names are hard coded into both the dictionaries and the query strings.

I'm using MethodBase or mapped object to pass parameter names to the SQL:

public static void CreateParameters(this DbCommand command, MethodBase method, params Object[] values)
{
        if(command == null) throw new ArgumentNullException("command");
        if(method == null) throw new ArgumentNullException("method");

        ParameterInfo[] paramInfo = method.GetParameters();
        for(Int32 loop = 0;loop < paramInfo.Length;loop++)
        {
            ParameterInfo parameter = paramInfo[loop];
            DbParameter dbParameter = command.CreateReflectedParameter(
                parameter.Name,
                parameter.IsOut,
                parameter.ParameterType.GetRealType(),
                parameter.IsOut ? null : values[loop]);

            command.Parameters.Add(dbParameter);
        }
}

Client:

private IEnumerable<KeyValuePair<String, Byte[]>> LoadPluginParameters(Int32 userId, Int32 applicationId, Int32 pluginId, String valueName)
{
    using(DbConnector connector = this.Plugin.SettingsInternal.CreateConnector())
    {
        connector.Command.CommandText = "settings.LoadPluginParameter";

        connector.CreateParameters(MethodInfo.GetCurrentMethod(),
            userId, applicationId, pluginId, valueName);

        connector.Connection.Open();
...

Or another solution with mapped object:

public static void CreateParameters(this DbCommand command, Object mappedObj)
{
    if(command == null) throw new ArgumentNullException("command");
    if(mappedObj == null) throw new ArgumentNullException("mappedObj");

    Type type=mappedObj.GetType();
    foreach(PropertyInfo property in type.GetProperties())
    {
        if(!property.IsDefined(typeof(XmlIgnoreAttribute), false))
        {
            DbParameter dbParameter = command.CreateReflectedParameter(
                property.Name,
                false,
                property.PropertyType.GetRealType(),
                property.GetValue(mappedObj, null));
            command.Parameters.Add(dbParameter);
        }
    }
}

Client:

private static void SavePluginParameterAsync(Object state)
{
    SaveParameterArgs args = (SaveParameterArgs)state;
    using(DbConnector connector = args.Data.Plugin.SettingsInternal.CreateConnector())
    {
        connector.Command.CommandText = "settings.SavePluginParameter";
        connector.CreateParameters(args);

        connector.Connection.Open();
        connector.Command.ExecuteNonQuery();
    }
}