1
\$\begingroup\$

I cannot edit the QueryPermissions class because it is an external .dll that was given to me to use. Yes, I realize it's a bad practice to use an out variables, but I am pushing for a rewrite of that service.

As you can see below the two methods do the almost the EXACT same thing only one uses a groupkey and the other uses a userkey and then calls the appropriate method. For whatever reason I am drawing a blank on refactoring these methods.

public IEnumerable<Node> GetNodesForUser(int userKey, IEnumerable<OnlineReport> onlineReports)
{
    var nodes = new List<Node>();
    foreach (var onlineReport in onlineReports)
    {
        var parentNode = new Node {Text = onlineReport.Key, Value = onlineReport.Value};
        var webQueries = new List<WebQuery>();
        //Line that is different
        _QueryPermissions.GetQueriesForTaskAndUser(Int32.Parse(onlineReport.Value), userKey, ref webQueries, true);
        foreach (var childNode in webQueries.Select(webQuery => new Node(parentNode){Text = webQuery.QueryTitle, Value = webQuery.QueryKey.ToString()}))
        {
            parentNode.Children.Add(childNode);
        }
        nodes.Add(parentNode);
    }
    return nodes;
}

public IEnumerable<Node> GetNodesForGroup(int groupKey, IEnumerable<OnlineReport> onlineReports)
{
    var nodes = new List<Node>();
    foreach (var onlineReport in onlineReports)
    {
        var parentNode = new Node { Text = onlineReport.Key, Value = onlineReport.Value };
        var webQueries = new List<WebQuery>();
        //Line that is different
         _QueryPermissions.GetQueriesForTaskAndUserGroup(Int32.Parse(onlineReport.Value), groupKey, ref webQueries, true);
        foreach (var childNode in webQueries.Select(webQuery => new Node(parentNode) { Text = webQuery.QueryTitle, Value = webQuery.QueryKey.ToString() }))
        {
            parentNode.Children.Add(childNode);
        }
        nodes.Add(parentNode);
    }
    return nodes;
}
\$\endgroup\$

2 Answers 2

5
\$\begingroup\$

The big difference between the functions is not the userKey vs. groupKey but rather than they have different queries embedded in them

_QueryPermissions.GetQueriesForTaskAndUser(Int32.Parse(onlineReport.Value), userKey, ref webQueries, true);

vs.

_QueryPermissions.GetQueriesForTaskAndUserGroup(Int32.Parse(onlineReport.Value), groupKey, ref webQueries, true);

It looks like these can be abstracted as

 delegate void NodeQueryDelegate(int reportValue, int key, ref List<WebQuery> webQueries, bool someBool);

so it should be possible to refactor thus

public IEnumerable<Node> GetNodesFromQuery(int key, IEnumerable<OnlineReport> onlineReports, NodeQueryDelegate query) {
    var nodes = new List<Node>();
    foreach (var onlineReport in onlineReports)  {
        var parentNode = new Node { Text = onlineReport.Key, Value = onlineReport.Value };
        var webQueries = new List<WebQuery>();
        //Line that is different
        query(Int32.Parse(onlineReport.Value), key, ref webQueries, true);
        foreach (var childNode in webQueries.Select(webQuery => new Node(parentNode) { Text = webQuery.QueryTitle, Value = webQuery.QueryKey.ToString() })) {
            parentNode.Children.Add(childNode);
        }
        nodes.Add(parentNode);
    }
    return nodes;
}

Getting the user nodes becomes

var userNodes = GetNodesFromQuery(userKey, onlineReports, _QueryPermissions.GetQueriesForTaskAndUser);

Getting the group node

var groupNodes = GetNodesFromQuery(groupKey, onlineReports, _QueryPermissions.GetQueriesForTaskAndUserGroup);
\$\endgroup\$
1
\$\begingroup\$

What about using a delegate?

private delegate void Query(int value, int key, ref List<WebQuery>, bool b);

public void SomeCallingMethod()
{
  IEnumerable<OnlineReport> reports;
  // one way
  int myUserKey;
  Query q = _QueryPermissions.GetQueriesForTaskAndUser;
  var returnValue = GetNodesForUser(q, myUserKey, reports);
  //or the other
  int groupKey;
  q = _QueryPermissions.GetQueriesForTaskAndUserGroup;
  returnValue = GetNodesForUser(q, groupKey, reports);
}

public IEnumerable<Node> GetNodesForUser(Query query, int key, IEnumerable<OnlineReport> onlineReports)
{
  var nodes = new List<Node>();
  foreach (var onlineReport in onlineReports)
  {
    var parentNode = new Node { Text = onlineReport.Key, Value = onlineReport.Value };
    var webQueries = new List<WebQuery>();
    //Line that is different
    // _QueryPermissions.GetQueriesForTaskAndUser(Int32.Parse(onlineReport.Value), userKey, ref webQueries, true);
    query.Invoke(Int32.Parse(onlineReport.Value), key, ref webQueries, true);
    foreach (var childNode in webQueries.Select(webQuery => new Node(parentNode) { Text = webQuery.QueryTitle, Value = webQuery.QueryKey.ToString() }))
    {
      parentNode.Children.Add(childNode);
    }
    nodes.Add(parentNode);
  }
  return nodes;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.