HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Dynamic Treenode Creation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
treenodecreationdynamic

Problem

I am trying to create a treenode structure based on a database table for a Dungeons and Dragons character creation program I am working on. I have come up with an incredibly confusing way to do it, but I am wondering if anyone can give me some better ideas on how to do this more efficiently. I have a tables called subfeats and feats. Subfeats contains all feats from the table feats that have subfeats. Really, all I want to do is create a parent/child treenode structure for all feats and their subfeats. Here is my code:

foreach (DataRow subrow in ds.subfeats.Rows)
{
    subfeatSet.Add(Convert.ToString(subrow[ds.subfeats.Columns.IndexOf("subfeat")]));
}

foreach (DataRow row in ds.feat.Rows)
{
    TreeNode newNode = new  TreeNode(Convert.ToString(row[ds.feat.Columns.IndexOf("name")]));

    if (subfeatSet.Contains(newNode.Text))
    {
        // Do Nothing.
    }
    else
    {
        treeviewFeatsFeats.Nodes.Add(newNode);
        foreach (DataRow subrow in ds.subfeats.Rows)
        {
            if (Convert.ToString(subrow[ds.subfeats.Columns.IndexOf("feat")]).Equals(newNode.Text))
            {
                TreeNode subTreeNode = new TreeNode(Convert.ToString(subrow[ds.subfeats.Columns.IndexOf("subfeat")]));
                newNode.Nodes.Add(subTreeNode);

                foreach (DataRow subsubrow in ds.subfeats.Rows)
                {
                    if (Convert.ToString(subsubrow[ds.subfeats.Columns.IndexOf("feat")]).Equals(subTreeNode.Text))
                    {
                        TreeNode subsubTreeNode = new TreeNode(Convert.ToString(subsubrow[ds.subfeats.Columns.IndexOf("subfeat")]));
                                subTreeNode.Nodes.Add(subsubTreeNode);
                    }
                }
            }
        }
    }
}

Solution

I like the use of white space and indentation, makes the code easy to read.

I would start using var for your variable declarations. It keeps the code cleaner, and easier for your eyes to scan.

Remove the // Do Nothing if statements. It is redundant and adds extra lines of code that are unneeded. This should work:

if (!subfeatSet.Contains(newNode.Text))
{
    treeviewFeatsFeats.Nodes.Add(newNode);
    foreach (DataRow subrow in ds.subfeats.Rows)
    {

    ...
}


I would also learn about linq, you can clean up most of your loops with linq. I would also look into moving some of the inner loops out into their own function. Moving to a function would remove some duplication you have in your code too.

private void ProcessNode(TreeNode parent, DataSet ds)
{
    var index = ds.subfeats.Columns.IndexOf("feat");
    foreach (var treeNode in (
            from row in ds.subfeats.Rows                
            let nodeName = Convert.ToString(row[index])
            where rows.Contains(nodeName)
            select new TreeNode(nodeName)))
    {
        ProcessNode(treeNode, rows);
        parent.Add(treeNode);            
    }
}


Now your function looks like this:

var index = ds.feat.Columns.IndexOf("name");
foreach (var treeNode in (
            from row in ds.feat.Rows,
            let nodeName = Convert.ToString(row[index])
            where rows.Contains(nodeName)
            select new TreeNode(nodeName)))
{
   ProcessNode(treeNode, ds);
}


This is a start. I'm not happy with having to pass DataSet into the functions, but without fully understanding the data structure, I can't think of another way of doing it.

Code Snippets

if (!subfeatSet.Contains(newNode.Text))
{
    treeviewFeatsFeats.Nodes.Add(newNode);
    foreach (DataRow subrow in ds.subfeats.Rows)
    {

    ...
}
private void ProcessNode(TreeNode parent, DataSet ds)
{
    var index = ds.subfeats.Columns.IndexOf("feat");
    foreach (var treeNode in (
            from row in ds.subfeats.Rows                
            let nodeName = Convert.ToString(row[index])
            where rows.Contains(nodeName)
            select new TreeNode(nodeName)))
    {
        ProcessNode(treeNode, rows);
        parent.Add(treeNode);            
    }
}
var index = ds.feat.Columns.IndexOf("name");
foreach (var treeNode in (
            from row in ds.feat.Rows,
            let nodeName = Convert.ToString(row[index])
            where rows.Contains(nodeName)
            select new TreeNode(nodeName)))
{
   ProcessNode(treeNode, ds);
}

Context

StackExchange Code Review Q#25459, answer score: 3

Revisions (0)

No revisions yet.