5
\$\begingroup\$

I want to create a Treeview in C# which will group file by prefix (here the prefix is a marked by the separator _). The following files should give this tree:

Files list:

p_a
p_a_test
p_LIG
p_p
p_p_c
p_p_c2
p_p_ccc
p_p_test
p_tres
TestLineGraph1
TestLineGrpah

Corresponding tree:

|--p_
    |--p_a
    |--p_a_test
    |--p_LIG
    |--p_p
    |--p_p_
        |--p_p_c
        |--p_p_c2
        |--p_p_ccc
        |--p_p_test
    |--p_tres
TestLineGraph1
TestLineGraph

And how I do it:

 private GraphUINode(List<string> subNodes, GraphUINode parent, string name, int lvl = 0)
        : base(parent.m_viewDataSubControl)
{
    parent.Nodes.Add(this);
    this.Name = name;
    this.Text = name;

    string currentPrefix = "";
    int pertinentSubNodes = 0;
    while (pertinentSubNodes < subNodes.Count -1 && subNodes[pertinentSubNodes].Split('_').Length < 2+ lvl)
        pertinentSubNodes++;

    for (int i = 0; i <= lvl; i++)
    {
        currentPrefix += subNodes[pertinentSubNodes].Split('_')[i] + "_";
    }
    List<String> children = new List<string>();
    foreach (string child in subNodes)
    {
        // The child is in the same group than the previous one
        if (child.StartsWith(currentPrefix))
        {
            children.Add(child);
        }
        else
        {
            // Create a node only if needed
            if (children.Count > 1)
            { 
                 // Create the new node
                new GraphUINode(children, this, currentPrefix, lvl + 1);
                children.Clear();
                children.Add(child);
            }
            else
            {
                if (children.Count == 1)
                {
                    new GraphTemplateNode(this, m_viewDataSubControl, children[0]);
                    children.Clear();
                }
                new GraphTemplateNode(this, m_viewDataSubControl, child);
            }
            currentPrefix = "";
            for (int i = 0; i <= lvl; i++)
            {
                currentPrefix += child.Split('_')[i] + "_";
            }                    
        }
    }
    if (children.Count == 1)
    {
        new GraphTemplateNode(this, m_viewDataSubControl, children[0]);
        children.Clear();
    }
}

but this looks very dirty. How can I improve it?

\$\endgroup\$
  • \$\begingroup\$ I like ANSII art. ++ \$\endgroup\$ – RubberDuck Dec 16 '14 at 18:56
  • 1
    \$\begingroup\$ @RubberDuck thanks dude, now I can call myself an artist! \$\endgroup\$ – Thomas Ayoub Dec 17 '14 at 9:59
4
\$\begingroup\$

Is this a bug ?

The correspondending tree you showed is not consistent. It should look like

|--p_
    |--p_a
    |--p_a_
          |--p_a_test  <- this
    |--p_LIG
    |--p_p
    |--p_p_
        |--p_p_c
        |--p_p_c2
        |--p_p_ccc
        |--p_p_test
    |--p_tres
TestLineGraph1
TestLineGraph

Style

  • You should always be consistent with the coding style you use. So if you omit (<-bad) braces {} for loops with a single statement, you should stick to it.

  • You shouldn't shorten variable / parameter names because that removes readability. -> lvl

Comments

Comments should describe why something is done. What is done should be described by the code itself by using meaningful names for classes, methods, parameters.

A comment which describes what is done like

 // Create the new node
new GraphUINode(children, this, currentPrefix, lvl + 1);  

should be removed.

Refactoring

Can't be done until the possibility of the bug as mentioned is cleared.

As the root problem to solve is to create a tree structure based on a List<string> I would extract this tree creation outside of the constructor to a separate method which only takes a List<string> as input parameter and is returning something like

class TreeNode
{
    public String Name { get; private set; }
    public IList<TreeNode> ChildNodes { get; private set; }
    public TreeNode()
    {
        ChildNodes = new List<TreeNode>();
    }
    public void FillChildNodes(IList<String> childNodes)
    {

    }
}  
\$\endgroup\$
  • \$\begingroup\$ in fact, as the category p_a_ contains only 1 child I don't consider it as a node (should I?). Thanks for the comment advice, I'll fix it in my code immediately! \$\endgroup\$ – Thomas Ayoub Dec 17 '14 at 10:21
  • \$\begingroup\$ I would consider it as a node. \$\endgroup\$ – Heslacher Dec 17 '14 at 10:24

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.