How can I refactor this method so I don't use it in multiple places?

Question

I have the following method which I use in multiple places, but it only differs by a few different things, so I was wondering how I can refactor it so I can have it in a common class and call it from there everywhere.

public override DataTable GetRecords(QueryContext queryContext, out int totalRecords)
{
    // Build Query
    //Different
    StringBuilder query = new StringBuilder("SELECT * FROM TABLE");
    Dictionary<string, string> parameters = new Dictionary<string, string>();

    if (queryContext.OrderByColumns.Count == 0)
    {
        //Can very in length or number of parameters
        queryContext.OrderByColumns.Add("param_1"); //different
        queryContext.OrderByColumns.Add("param_2"); //different
    }

    if (queryContext.Parameters.Count > 0)
    {
        foreach (QueryParameter p in queryContext.Parameters)
        {
            dataAccess.paramAdd(parameters, query, p.ColumnName, p.Value.ToString());
        }
    }

    // Order By Clause
    query.Append(queryContext.OrderByColumns.GetSqlClause());

    // Apply Limit
    if (queryContext.ApplyLimit)
    {
        query.AppendFormat(" LIMIT {0},{1}", queryContext.Offset, queryContext.Limit);
    }

    //Execute the query.
    DataSet results = dataAccess.ExecuteQuery(query.ToString(), parameters);
    totalRecords = Convert.ToInt32(results.Tables[1].Rows[0][0]);
    return results.Tables[0];
}

The only differences in other places are the value of the query variable and the paramters added by queryContext.OrdByColumn.Add(...). Other than that, everything is the same.

My first shot was going to be doing something like:

public override DataTable GetRecords(StringBuilder query, string[] orderByParams, QueryContext queryContext, out int totalRecords)
{

    Dictionary<string, string> parameters = new Dictionary<string, string>();

    if (queryContext.OrderByColumns.Count == 0)
    {
        foreach(var param in orderByParams)
        {
            queryContext.OrderByColumns.Add(param);
        }
    }

    if (queryContext.Parameters.Count > 0)
    {
        foreach (QueryParameter p in queryContext.Parameters)
        {
            dataAccess.paramAdd(parameters, query, p.ColumnName, p.Value.ToString());
        }
    }

    // Order By Clause
    query.Append(queryContext.OrderByColumns.GetSqlClause());

    // Apply Limit
    if (queryContext.ApplyLimit)
    {
        query.AppendFormat(" LIMIT {0},{1}", queryContext.Offset, queryContext.Limit);
    }

    //Execute the query.
    DataSet results = dataAccess.ExecuteQuery(query.ToString(), parameters);
    totalRecords = Convert.ToInt32(results.Tables[1].Rows[0][0]);
    return results.Tables[0];
}

LINQ is available to me, so if that can improve it, I am welcome to ideas using that too.

Answer 1

If it's supposed to be used globally, it shouldn't be making decisions on what to add to the queryContext. That is a parameter so the caller should be adding those parameters as necessary. I don't know where dataAccess comes form but again, that should be done outside. That leaves you with little left.

public override DataTable GetRecords(WhateverClassThisIs dataAccess, QueryContext queryContext, out int totalRecords)    
{
    // Build Query
    StringBuilder query = new StringBuilder("SELECT * FROM TABLE");
    Dictionary<string, string> parameters = new Dictionary<string, string>();

    // Order By Clause
    query.Append(queryContext.OrderByColumns.GetSqlClause());

    // Apply Limit
    if (queryContext.ApplyLimit)
    {
        query.AppendFormat(" LIMIT {0},{1}", queryContext.Offset, queryContext.Limit);
    }

    //Execute the query.
    DataSet results = dataAccess.ExecuteQuery(query.ToString(), parameters);
    totalRecords = Convert.ToInt32(results.Tables[1].Rows[0][0]);
    return results.Tables[0];
}

That's my recommendation.

If it's possible to pull from a different table or something other than * then that should be handled with arguments to the function as well, if not build the base string before calling this function. You did name it GetRecords and so by that all it should do is fetch records based on what you pass it. It shouldn't dynamically build a query based on the status of the arguments, it should just use the arguments to build the query without (or with very little) decision making.

EDIT

In order to make this a utility function it needs to perform the most generic process you can make it perform. To add any kind of specificity to this function will make it less of a global utility and more of a specialized utility (which is fine, if that's really what you want - but general is better and more reusable). I mentioned in the original body of the answer and in comments that you need to handle you specialized order by parameters in the caller and not in this function. After all, you are already passing the queryContext to this function. Here's a sample of the caller using what I'm discussing:

private void ICallUtiltyFunction()
{
    QueryContext context = new QueryContext(); // I don't know if this is really how to instantiate this object

    // Add these here, because this function knows what it needs and
    // it already created the queryContext object
    context.OrderByColumns.Add("param_1");
    context.OrderByColumns.Add("param_2"); 

    // I also know my limit and start here
    context.Offset = 30;
    context.Limit = 30;

    int totalRecords = 0; // You really don't need this, it's wasteful
    DataTable results = Utilities.GetRecords(dataAccess, context, totalRecords);

    // Use the results now
}

Answer 2

You can take an array of strings for the different query params, like:

public override DataTable GetRecords(QueryContext queryContext, string[] queryParams, out int totalRecords)
{
    // Build Query
    //Different
    StringBuilder query = new StringBuilder("SELECT * FROM TABLE");
    Dictionary<string, string> parameters = new Dictionary<string, string>();

    if (queryContext.OrderByColumns.Count == 0)
    {
        foreach(string param in queryParams) {
           queryContext.OrderByColumns.Add(param);
         }
    }

   ...

}

Answer 3

I would pass the string to create the query in one variable, then a list of strings to go into your queryContext.OrderByColumns.Add("") code.

public override DataTable GetRecords(QueryContext queryContext, String queryString, List<String> parameters, out int totalRecords)
{
    StringBuilder query = new StringBuilder(queryString);

    if (queryContext.OrderByColumns.Count == 0)
    {
        foreach(String str in params)
        {
            queryContext.OrderByColumns.Add(str);
        }
    }

    .....

}

Answer 4

Pass the query variable and the param_1, param_2, etc. as parameters to the method. If there is a varying number of parameters to pass to queryContext.OrderByColumns.Add, I'd pass them in some kind of list and loop through them, rather than creating overrides of the method for each number of parameters needed.