2
\$\begingroup\$

I'm trying to create a function to return a DataTable with the results of a MySQL query where the program doesn't know the names of the queried columns until run-time.

How can I improve the readability of this function? Is concatenating strings like this an appropriate way to generate MySQL queries?

public static DataTable Entries(string db, string user, string dtStart, string dtEnd = "yyyy-MM-dd HH:mm:ss", List<string[]> column = null)
    {
            string mysqlTimestamp = "yyyy-MM-dd HH:mm:ss";
            string cmd = string.Format("SELECT * FROM `{0}`.`{1}_table` WHERE ", db, user);

            if (dtEnd != mysqlTimestamp)
            {
                cmd += "`Timestamp` >= @dtStart AND `Timestamp` <= @dtEnd";
            }
            else {
                cmd += "`Timestamp` = @dtStart";
            }

            if (column != null)
            {
                cmd += " AND";

                string colQuery = " `{0}` = @{0}";
                for (int i = 0; i < column.Count(); i++)
                {
                    cmd += string.Format(colQuery, column[i]);
                    if (i != column.Count() - 1) { cmd += " AND"; }
                }
            }

            var data = new DataSet();
            using (var MySql = Query.Connection(Session.Current.Username, Session.Current.Database))
            {
                MySql.Open();
                using (var MySqlCmd = new MySqlCommand(cmd, MySql))
                {
                    MySqlCmd.Parameters.AddWithValue("@dtstart", dtStart);
                    MySqlCmd.Parameters.AddWithValue("@dtEnd", dtEnd);
                    if (column != null)
                    {
                        foreach (string[] col in column)
                        {
                            MySqlCmd.Parameters.AddWithValue("@" + col[0], col[1]);
                        }
                    }

                    using (var Adapter = new MySqlDataAdapter(MySqlCmd))
                    {
                        Adapter.Fill(data);
                    }
                }
            }
            return data.Tables[0];
        }
    }

Results in the string:

SELECT * FROM `db`.`someuser_table` WHERE `Timestamp` >= @dtStart AND `Timestamp` <= @dtEnd AND `Column1` = @Column1 AND `Column3` = @Column3

or

SELECT * FROM `db`.`someuser_table` WHERE `Timestamp` >= @dtStart

and anything in between.

\$\endgroup\$
1
\$\begingroup\$
  • To improve readability you should use proper naming conversions.
  • From the given code what i can understand that the input parameter user is for constructing the table name, that means all your table looks like someName_table(i'm not sure), if so user is not a good name for that variable, let me rename them as queryOnTable.
  • Another thing i have noticed is that Timestamp will be a common column for all your tables.(me too assume so or else we have to consider that too).
  • Here in your case you are specifying the database name as well as the current user of the database in the connection using var MySql = Query.Connection(Session.Current.Username, Session.Current.Database) so you need not be specified them in the executable query. Or else you can use the input parameters db and user(if the above point is false) can be used to create the connection(if connection string change dynamically). This may help you to reduce the complex formatting of the select statement.
  • Here the given snippet will always select the whole data ( select * ) from the given table, This will not be a good choice, always collect the data which you want. I i prefer you to use another input array to the function that specifies the required fields.
  • This article says that .Parameters.Add() is better choice that .Parameters.AddWithValue() So we can prefer that here by changing the last parameter List<string[]> column to something like List<myQueryParameter>.

where myQueryParameter is defined as:

public class myQueryParameter
    {
        public MySqlDbType  paramType { get; set; }
        public string paramValue { get; set; }
    }
  • I suggest you to include an additional parameter called additionalClause which can be used for specifying clauses like Group by, order by etc.

  • Now the query building; Many article available in the various sites stated that StringBuilder is the best option for concatenating the string when the concatenations are more than 1000, where as String.Join() is even more efficient than StringBuilder if concatenations are less than 1000. + and String.Concat() are less efficient than these two.

Now consider the modified method:

public static DataTable GetDataTable(string queryOnTable,
                                    string dtStart,
                                    string dtEnd = "yyyy-MM-dd HH:mm:ss",
                                    List<string> reqColumns = null,
                                    List<myQueryParameter> conditionalParams = null),
                                    string additionalClause="")
    {
        if (reqColumns == null) { reqColumns = new List<string>() { "*" }; } // select all columns if not specified

        string mysqlTimestamp = "yyyy-MM-dd HH:mm:ss";

        StringBuilder sqlQueryBuilder = new StringBuilder();
        sqlQueryBuilder.Append("SELECT ");
        sqlQueryBuilder.Append(String.Join(",", reqColumns));
        sqlQueryBuilder.Append(String.Format(" FROM {0} WHERE ", queryOnTable));

        if (dtEnd != mysqlTimestamp)
        {
            sqlQueryBuilder.Append("`Timestamp` >= @dtStart AND `Timestamp` <= @dtEnd");
        }
        else
        {
            sqlQueryBuilder.Append("`Timestamp` = @dtStart");
        }

        if (conditionalParams != null)
        {
            foreach (myQueryParameter param in conditionalParams)
            {
                sqlQueryBuilder.Append(String.Format(" AND `{0}` = @{0}", param.param));
            }
        }

        if (additionalClause != "")
            sqlQueryBuilder.Append(additionalClause); 

        var data = new DataSet();
        using (var MySql = new MySqlConnection("Query.Connection(Session.Current.Username, Session.Current.Database)"))
        {
            MySql.Open();
            using (var MySqlCmd = new MySqlCommand(sqlQueryBuilder.ToString(), MySql))
            {
                MySqlCmd.Parameters.AddWithValue("@dtstart", dtStart);
                MySqlCmd.Parameters.AddWithValue("@dtEnd", dtEnd);
                if (conditionalParams != null)
                {
                    foreach (myQueryParameter condition in conditionalParams)
                    {
                        MySqlCmd.Parameters.Add(String.Format("@{0}",condition.param),condition.paramType).Value = condition.paramValue; 
                    }
                }
                using (var Adapter = new MySqlDataAdapter(MySqlCmd))
                {
                    Adapter.Fill(data);
                }
            }
        }
        return data.Tables[0];
    }

Before going to the usage example let me define a table; Let user_data is the table having columns as follows:

Column Name  | DataType
user_id          VarChar
user_name        VarChar
int_field        Int32
text_field1       Text
text_field2       Text
text_field3       Text
Timestamp         DateTime

We can call the GetDataTable() method as like the following:

// Creating input parameters
List<string> reqcolumns = new List<string>() { "user_name", "int_field", "text_field1", "text_field2","text_field3" };
List<myQueryParameter> parameters = new List<myQueryParameter>();
parameters.Add(new myQueryParameter() { param = "user_id", paramType = MySqlDbType.VarChar, paramValue = "USR001" });
parameters.Add(new myQueryParameter() { param = "int_field", paramType = MySqlDbType.Int32, paramValue = "12" });
parameters.Add(new myQueryParameter() { param = "user_name", paramType = MySqlDbType.VarChar, paramValue = "un-luckey" });

//Calling the method
var dt = GetDataTable("user_data",
                      "2014-02-15 00:00:00",
                      "yyyy-MM-dd HH:mm:ss",
                      reqcolumns, parameters,
                      " Order by Timestamp");

Which will creates the query as like the following:

SELECT user_name,int_field,text_field1,text_field2,text_field3 FROM user_data WHERE Timestamp = @dtStart AND user_id = @user_id AND int_field = @int_field AND user_name = @user_name Order by Timestamp

\$\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.