Pregunta

I have a form that has a set of labels that can be clicked - See Screenshot (Labels 93 to Label 21 on the Player side and labels 10 to 22 on the computer side).

enter image description here

All I want to do is change the label back colour when the control is clicked. This I have figured out.

My question is more about clean code. Instead of having

private void Playerlabel_Click(object sender, EventArgs e)
      {

            lblPlayerCardInt.BackColor = Color.FromArgb(219, 255, 248);

      }

private void Playerlabel2_Click(object sender, EventArgs e)
      {

            lblPlayerCardStrength.BackColor = Color.FromArgb(219, 255, 248);

      }

//repeat 5 times and then another 5 times for the computer labels.

Is there a way of defining a method so that no matter if I click the label or the computer clicks it, the method of highlighting the label is called.

I'm think something along the lines of

public void HighlightLabel()
{
    foreach (Control x in this.Controls)
      {
          if (x is Label)
              {
                  ((Label)x).BackColor = Color.FromArgb(219, 255, 248);
              }
      }
}

Is this the right approach? It may seem like an obvious question, but new to programming in C#/OOP so want to be sure I'm doing it the correct way.

¿Fue útil?

Solución

Yes you can set only 1 Click event-handler for all those labels:

Step 1:

private void lbl_Click(object sender, EventArgs e)
{
    Label lbl = sender as Label;
    lbl.BackColor = Color.FromArgb(219, 255, 248);    
}

Step 2:

If you create labels on design time then do the following for each label:

  1. On Form designer, select label then open View > Properties > Events tab
  2. Select above lbl_Click event-handler from the dropdown list

OR

If your create labels programatically then use this instead:

lblPlayerCardInt.Click += new EventHandler(lbl_Click);
lblPlayerCardStrength.Click += new EventHandler(lbl_Click);
...

Otros consejos

Seeing as all you're doing is setting the backcolor each time why not just have 1 method and do something like:

private void label_Click(object sender, EventArgs e)
{
    Label mylabel = (Label) sender;
    mylabel.backcolor = Color.FromArgb(219, 255, 248);
}

You can then just put Label_Click in as the click event for each label rather than doing a separate method for each label.

However, if you want to do more based on the label you can use the sender to determine the label clicked. Something like this:

private void label_Click(object sender, EventArgs e)
{
    Label mylabel = (Label) sender;
    //Determine which label has been clicked by name or id for example
    switch(myLabel.Name)
    {
        case "lblPlayerCardint":
            //Do something
            break;
    }        
}

You could cast the event sender object to a label

private void label_Click(object sender, EventArgs e)
 {
             Label lblClicked=sender as Label;
           lblClicked.BackColor = Color.FromArgb(219, 255, 248);

 }

You could use OfType which is part of the System.Linq namespace:

foreach(Label lbl in this.Controls.OfType<Label>())
    lbl.BackColor  = Color.FromArgb(219, 255, 248);

This should do the trick for you and register the same click event for all your labels on the form.

    this.Controls
        .OfType<Label>()
        .AsParallel()
        .ForAll(x => x.Click += (o,e) => x.BackColor= Color.FromArgb(219, 255, 248));
Licenciado bajo: CC-BY-SA con atribución
No afiliado a StackOverflow
scroll top