重构这段代码

Question

我正在尝试根据来自各个部门的工作人员列表（lstStaff）填充树视图。

public class Staff
{
    public int StaffID { get; set; }
    public string StaffName { get; set; }
    public int DeptID { get; set; }
    public string DepartmentName { get; set; }
}

结果应该是这样的：

Department A

• John
• Dave

Department B

• Andy
• Leon

我编写了以下代码。不过，我认为代码应该进一步重构。

RadTreeNode deptNode;
RadTreeNode staffNode;
var departments = (from d in lstStaff
                   where !string.IsNullOrEmpty(d.DepartmentName) 
                select new { d.DeptId, d.DepartmentName })
                .Distinct();

foreach (var d in departments)
{
    //add department node
    deptNode = new RadTreeNode()
    {
        Value = d.DeptId,
        Text = d.DepartmentName
    };
    tvwDepartmentCases.Nodes.Add(deptNode);

    //add staff nodes to department node
    var staffs = lstStaff
                    .ToList()
                    .Where(x => x.DeptId == d.DeptId);

    foreach (var s in staffs)
    {
        staffNode = new RadTreeNode()
        {
            Value = s.StaffID,
            Text = s.StaffName
        };
        deptNode.Nodes.Add(staffNode);
    }
}

欢迎任何评论。谢谢！

Answer 1

我认为你所拥有的内容非常简单且易于阅读。它有性能改进的空间（可能可以忽略不计），但你会牺牲一些可读性。话虽如此，您对当前解决方案有哪些不满意而希望进一步重构？

Answer 2

我现在必须跑去上班，但我认为你在循环中做了太多查询。您应该在循环之前使用.ToLookup为员工组创建查找表，然后使用该查找表来查找每个部门的员工。

Answer 3

我会写 -

foreach(var member in lstStaff)
 var deptNode = FindDeptNode(member.DeptId, member.DeptName);
 deptNode.Add(member.StaffId, member.StaffName);


.. // in FindDeptNode

if(!departmentsTreeView.ContainssNode(deptId, deptName))
 departmentsTreeView.Add(deptId, deptName);

return departmentsTreeView[deptId, deptName];

你可以将其转换为可编译的形式。

干杯。

Answer 4

我会更进一步，从查询中创建人员节点。这在逻辑上对我来说更清晰，因为我们拥有此时创建节点所需的所有信息。不幸的是，不可能以干净、可读的方式在查询中创建部门节点。

var depts = from staff in lstStaff
            where !string.IsNullOrEmpty(staff.DepartmentName)
            group   //staff nodes
                new RadTreeNode  //each item in the group is a node
                {
                    Value = staff.StaffID,
                    Text = staff.StaffName,
                }
            by      //departments
                new
                {
                    staff.DeptID,
                    staff.DepartmentName,
                };
foreach (var dept in depts)
{
    var deptNode = new RadTreeNode
    {
        Value = dept.Key.DeptID,
        Text = dept.Key.DepartmentName,
    };
    deptNode.Nodes.AddRange(dept.ToArray());
    tvwDepartmentCases.Nodes.Add(deptNode);
}

Answer 5

使用GroupBy 运算符，它很简单，而且看起来好多了。

var departmentGroups = lstStaff.GroupBy(staff => 
    new { staff.DeptID, staff.DepartmentName });

foreach (var department in departmentGroups)
{
    var deptNode = new RadTreeNode()
    {
        Value = department.Key.DeptID,
        Text = department.Key.DepartmentName
    };

    tvwDepartmentCases.Nodes.Add(deptNode);

    foreach (var staffMember in department)
    {
        var staffNode = new RadTreeNode()
        {
            Value = staffMember.StaffID,
            Text = staffMember.StaffName
        };
        deptNode.Nodes.Add(staffNode);
    }
}

它还降低了复杂性，因为您不需要遍历每个部门的整个 lstStaff 集合。

Answer 6

var grping = from staff in lstStaff
             where !String.IsNullOrEmpty(staff.StaffName)
             group staff.StaffName by staff.DepartmentName;

foreach (var deptGrp in grping)
{
         //add a high level node using deptGrp.Key
         foreach (var staffName in deptGrp) 
               //add a lower level node
}