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.
1 Answer 1
- 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 likesomeName_table
(i'm not sure), if souser
is not a good name for that variable, let me rename them asqueryOnTable
. - 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 parametersdb
anduser
(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 parameterList<string[]> column
to something likeList<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 asString.Join()
is even more efficient thanStringBuilder
if concatenations are less than 1000.+
andString.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