views:

54

answers:

1

Hi All

I wrote a small code to generate ToC or Hierachical Bullets like one sees in a word document. Like the following set

 1. 
 2 
 3
  3.1
    3.1.1 
 4 
 5

and so on so forth, the code is working but I am not happy with the code I have written, I would appreciate if you guys could shed some light on how I can improve my C# code. You can download the project from Rapidshare

Please do let me know if you need more info. I am making this a community wiki.

        private void frmMain_Load(object sender, EventArgs e)
    {
        this.EnableSubTaskButton();
    }

    private void btnNewTask_Click(object sender, EventArgs e)
    {
        this.AddNodes(true);
    }

    private void AddNodes(bool IsParent)
    {
        TreeNode parentNode = tvToC.SelectedNode;
        string curNumber = "0";

        if (parentNode != null)
        {
            curNumber = parentNode.Text.ToString();
        }

        curNumber = this.getTOCReference(curNumber, IsParent);

        TreeNode childNode = new TreeNode();
        childNode.Text = curNumber;

        this.tvToC.ExpandAll();
        this.EnableSubTaskButton();
        this.tvToC.SelectedNode = childNode;

        if (IsParent)
        {
            if (parentNode == null)
            {
                tvToC.Nodes.Add(childNode);
            }
            else
            {
                if (parentNode.Parent != null)
                {
                    parentNode.Parent.Nodes.Add(childNode);
                }
                else
                {
                    tvToC.Nodes.Add(childNode);
                }
            }   
        }
        else
        {
            parentNode.Nodes.Add(childNode);
        }
    }

    private string getTOCReference(string curNumber, bool IsParent)
    {
        int lastnum = 0;
        int startnum = 0;
        string firsthalf = null;

        int dotpos = curNumber.IndexOf('.');

        if (dotpos > 0)
        {
            if (IsParent)
            {
                lastnum = Convert.ToInt32(curNumber.Substring(curNumber.LastIndexOf('.') + 1));
                lastnum++;                    
                firsthalf = curNumber.Substring(0, curNumber.LastIndexOf('.'));
                curNumber = firsthalf + "." + Convert.ToInt32(lastnum.ToString());
            }
            else
            {
                lastnum++;
                curNumber = curNumber + "." + Convert.ToInt32(lastnum.ToString());
            }
        }
        else
        {
            if (IsParent)
            {
                startnum = Convert.ToInt32(curNumber);
                startnum++;
                curNumber = Convert.ToString(startnum);
            }
            else
            {
                curNumber = curNumber + ".1";
            }
        }
        return curNumber;
    }

    private void btnSubTask_Click(object sender, EventArgs e)
    {
        this.AddNodes(false);
    }

    private void EnableSubTaskButton()
    {
        if (tvToC.Nodes.Count == 0)
        {
            btnSubTask.Enabled = false;
        }
        else
        {
            btnSubTask.Enabled = true;
        }
    }

    private void btnTest1_Click(object sender, EventArgs e)
    {
        for (int i = 0; i < 100; i++)
        {
            this.AddNodes(true);
        }
        for (int i = 0; i < 5; i++)
        {
            this.AddNodes(false);
        }
        for (int i = 0; i < 5; i++)
        {
            this.AddNodes(true);
        }
    }
A: 

Although I know nothing about C#, here are some thoughts on the code you posted (can't access the rapidshare file):

  • there are no comments
  • sometimes you use this before a method call, sometimes you don't
  • some variable names are capitalized (IsParent), some aren't (curNumber)
  • some method names are capitalized (AddNodes()), some aren't (btnNewTask_Click())
  • even in short methods, I'd avoid single-character variable names (e)
  • you could change EnableSubTaskButton() to be a one-liner
JRL
"btnNewTask_Click()","e" are IDE generated although that's not an excuse for me to follow the same convention. Do you think the logic can be improved or if there flaws in it ?
vikramjb