"Clean" way to modify the "Visible" attribute of a Picturebox

Asked

Viewed 438 times

6

I have 5 background images for my application and that will be visible to the user’s taste. What is the simplest way (clean code) to get the user to choose?

    private void radioButton1_CheckedChanged(object sender, EventArgs e)
    {
        pictureBox1.Visible = true;
        pictureBox2.Visible = false;
        pictureBox3.Visible = false;
        pictureBox4.Visible = false;
        pictureBox5.Visible = false;
    }
    private void radioButton2_CheckedChanged(object sender, EventArgs e)
    {
        pictureBox2.Visible = true;
        pictureBox1.Visible = false;
        pictureBox3.Visible = false;
        pictureBox4.Visible = false;
        pictureBox5.Visible = false;
    }
    private void radioButton3_CheckedChanged(object sender, EventArgs e)
    {
        pictureBox1.Visible = false;
        pictureBox2.Visible = false;
        pictureBox3.Visible = true;
        pictureBox4.Visible = false;
        pictureBox5.Visible = false;
    }
    private void radioButton4_CheckedChanged(object sender, EventArgs e)
    {
        pictureBox1.Visible = false;
        pictureBox2.Visible = false;
        pictureBox3.Visible = false;
        pictureBox4.Visible = true;
        pictureBox5.Visible = false;
    }
    private void img5_CheckedChanged(object sender, EventArgs e)
    {
        pictureBox1.Visible = false;
        pictureBox2.Visible = false;
        pictureBox3.Visible = false;
        pictureBox4.Visible = false;
        pictureBox5.Visible = true;
    }

4 answers

8

The Response of the Gypsy Morrison Mendez is good and solves the problem. But one may have a problem where one cannot change the structure of the methods. Still you have a solution:

private void ChangePictureBoxes() {
    pictureBox1.Visible = false;
    pictureBox2.Visible = false;
    pictureBox3.Visible = false;
    pictureBox4.Visible = false;
    pictureBox5.Visible = false;
}

private void radioButton1_CheckedChanged(object sender, EventArgs e) {
    ChangePictureBoxes();
    pictureBox1.Visible = true;
}
private void radioButton2_CheckedChanged(object sender, EventArgs e) {
    ChangePictureBoxes()
    pictureBox2.Visible = true;
}
private void radioButton3_CheckedChanged(object sender, EventArgs e) {
    ChangePictureBoxes()
    pictureBox3.Visible = true;
}
private void radioButton4_CheckedChanged(object sender, EventArgs e) {
    ChangePictureBoxes()
    pictureBox4.Visible = true;
}
private void img5_CheckedChanged(object sender, EventArgs e) {
    ChangePictureBoxes()
    pictureBox5.Visible = true;
}

Eventually it is possible to use the utility method the way Sunstreaker suggested, as long as you know all the PictureBox should receive that state. In fact if the most appropriate semantics is "all PictureBoxes" instead of "the PictureBoxes tal and tal", this would be a more generic and lasting solution, despite possibly having a loss of performance but probably irrelevant.

private void ChangePictureBoxes() {
    foreach (var p in Controls.OfType<PictureBox>()) p.Visible = false;
}

I put in the Github for future reference.

I am not a fan of the term attribute in this context, I prefer field and demonstrate why in What is the difference between attribute and field in the classes?.

7

You can take all(if that’s not a problem for you) the components of the type PictureBox and put them in a collection, and in a loop change property Visible for false, and then on PictureBox target change to true the property Visible.
Something like that:

   var pictureBoxes = Controls.OfType<PictureBox>();
   foreach (var p in pictureBoxes) p.Visible = false;
   pictureBox1.Visible = true;

Do it at the event CheckedChanged() of all its RadioButton's only by changing the name of pictureBox target.

  • There’s only one problem: what if there’s more than five PictureBox in the Form and the others are not modified?

  • @I don’t think that’s gonna be a problem, at least not for the OP, according to him, there are only 5 picturebox.

  • 2

    Do you accept the suggestion? How about minimizing your response to a single event?

  • 1

    Now yes. Excellent. + 1.

4


Thus:

private void radioButtonUnico_CheckedChanged(object sender, EventArgs e)
{
    pictureBox1.Visible = false;
    pictureBox2.Visible = false;
    pictureBox3.Visible = false;
    pictureBox4.Visible = false;
    pictureBox5.Visible = false;

    switch (((RadioButton)sender).Name) {
        case "radioButton1": 
            pictureBox1.Visible = true;
            break;
        case "radioButton2": 
            pictureBox2.Visible = true;
            break;
        case "radioButton3": 
            pictureBox3.Visible = true;
            break;
        case "radioButton4": 
            pictureBox4.Visible = true;
            break;
        case "radioButton5": 
            pictureBox5.Visible = true;
            break;
    }
}

Set the event for all your RadioButtons.

0

Whereas control is being used RadioButton next to the event CheckedChanged, we could say that there is a problem even of logic with your code.

If any other part of your system changes the state of the property checked for false and its control was checked, the event CheckedChanged would be fired, resulting in something unwanted... It is important to remember that events are not only triggered by user actions via UI.

I leave as suggestion the code below.

Create a method that controls the visibility of PictureBoxes property-based checked of each RadioButon related to them.

private void ChangePictureBoxes()
{
    pictureBox1.Visible = radioButton1.Checked;
    pictureBox2.Visible = radioButton2.Checked;
    pictureBox3.Visible = radioButton3.Checked;
    pictureBox4.Visible = radioButton4.Checked;
    pictureBox5.Visible = radioButton5.Checked;    
}

Call this method at each of your event RadioButton:

private void radioButton1_CheckedChanged(object sender, EventArgs e)
{
    ChangePictureBoxes();
}

Browser other questions tagged

You are not signed in. Login or sign up in order to post.