Search code examples
c#.netwinforms.net-4.6.1

How to assign text to a Label with the Label ID being assigned by a string?


Target:
To write in the label text, but the label ID is assigned by a string.

Problem:
It doesn't work, no errors given. I looked in most places for an answer but nothing helped!

My Code:

string asdfj = treeView2.SelectedNode.Text;
string adqien = System.IO.Path.Combine(dir7, asdfj);
string[] tnsop = System.IO.File.ReadAllLines(@adqien);
h1a.Text = "100";
for (int o = 2; o > 6; o++)
{
    //This is the label name e.g "h2a',h3a" etc
    string tempc = string.Format("h" + o.ToString() + "a");

    foreach (Control ctr in this.Controls)
    {
        if (ctr is Label)
        {
            if (ctr.Name == tempc)
            {
                ctr.Text = tnsop[o];
            }
        }
    }
}

I also consulted this post:
Use string variable content as label ID to update label.Text, I get error - 'string' does not contain a definition for 'Text'


Solution

  • The for loop

    First of all, this is wrong for (int o = 2; o > 6; o++).

    It starts at o = 2, then checks if o > 6, which is false, because o = 2, and then skips the loop.

    I guess you wanted to write: for (int o = 2; o < 6; o++). I am not sure about that, fix it as appropiate.

    Addendum: This would have been easily discovored by debugging and stepping. You can start by adding a break point on your code (in Visual Studio you can place your cursor on the desired line and press F9 - by default) and then run in the debugger. When the a line with the break point is reached, the debbuger stops the execution and let you inspect the values of the variables, the call stack, etc. You can then step with F10 or F11 (if you want to inside a method call) and see how the code evolves. You would notice it does not enter the for loop.


    Finding the labels

    If finding the label still does not work, I would guess the problem is that the labels are not directly on the form or that they do not have the given name.

    You can use this.Controls.Find(name, searchAllChildren) to get the labels you need.

    That is:

    string labelName = string.Format("h" + o.ToString() + "a");
    Control[] control = this.Controls.Find(labelName, true);
    

    Note: Yes, I can figure out it is the name of the label by how you use it. Using a comment to tell me saves some time... however, please use better variable names. You won't need a comment to tell me this is the name of the label if the variable name says so.

    You still need to check it for the label:

    string labelName = string.Format("h" + o.ToString() + "a");
    Control[] controls = this.Controls.Find(labelName, true);
    foreach (Control control in controls)
    {
        if (control is Label) // if (control.GetType() == typeof(Label))
        {
           // ...
        }
    }
    

    Building a Dictionary

    However, I would advice against doing this every time. Instead, I suggest to build a dictionary:

    Dictionary<string, Label> labels;
    
    // ...
    
    labels = new Dictionary<string, Label>();
    
    for(int o = 2; o < 6; o++)
    {
        string labelName = string.Format("h" + o.ToString() + "a");
        Label label = GetLabel(labelName);
        labels.Add(labelName, label);
    }
    
    // ...
    
    private Label GetLabel(string labelName)
    {
        Control[] controls = this.Controls.Find(labelName, true);
        foreach (Control control in controls)
        {
            if (control is Label) // if (control.GetType() == typeof(Label))
            {
               return control as Label;
            }
        }
        return null;
    }
    

    Note: I suggest to make the dictionary a field and initialize it only once during the form load.

    This separates the responsability of finding the labels from reading the file (which is external to the program). Allowing you to test if it can find the right controls without the need of a file.

    It will also make the case where the Label is not found visible (we just added a null to the dictionary).

    And then use it:

    string[] tnsop = System.IO.File.ReadAllLines(@adqien);
    for (int o = 2; o < 6; o++)
    {
        string labelName = string.Format("h" + o.ToString() + "a");
        label = labels[labelName];
        label.Text = tnsop[o];
    }
    

    The code above should throw NullReferenceException if the label was not found while building the dictionary.


    Simplify

    I guess we can do better. The designer will create fields for your labels, you can just add them to the dictionary:

    Dictionary<string, Label> labels;
    
    // ...
    
    labels = new Dictionary<string, Label>();
    labels["h2a"] = h2a;
    labels["h3a"] = h3a;
    labels["h4a"] = h4a;
    labels["h5a"] = h5a;
    
    // ...
    
    string[] tnsop = System.IO.File.ReadAllLines(@adqien);
    for (int o = 2; o < 6; o++)
    {
        string labelName = string.Format("h" + o.ToString() + "a");
        label = labels[labelName];
        label.Text = tnsop[o];
    }
    

    Note: There are plenty of opportunities for more modern syntax, including collection initialization and the var keyword.

    Addendum: I am unrolling the loop in the above code, this is ok for maintainability if the number of iterations small, in fact, it is a common optimization. You could, in theory do it for the other loop too.


    PS. An array will do

    I noticed, after finishing writing, that the code only needs to look up by an int o.

    We can rework to use int for dictionary keys:

    Dictionary<int, Label> labels;
    
    // ...
    
    labels = new Dictionary<int, Label>();
    labels[2] = h2a;
    labels[3] = h3a;
    labels[4] = h4a;
    labels[5] = h5a;
    
    // ...
    
    string[] tnsop = System.IO.File.ReadAllLines(@adqien);
    for (int o = 2; o < 6; o++)
    {
        label = labels[o];
        label.Text = tnsop[o];
    }
    

    Now we have less concatenations, and a simpler code.

    We could, in fact, be using an array:

    Label[] labels;
    
    // ...
    
    labels = new Label[4];
    labels[0] = h2a;
    labels[1] = h3a;
    labels[2] = h4a;
    labels[3] = h5a;
    
    // ...
    
    string[] tnsop = System.IO.File.ReadAllLines(@adqien);
    for (int o = 2; o < 6; o++)
    {
        label = labels[o - 2];
        label.Text = tnsop[o];
    }
    

    Notice I did offset the indexes to be able to use the array from index 0.