1
\$\begingroup\$

I'm trying to alternate if a button is checked. I've come up with this, which works but its feels unnecessary to write 70 lines of code for such a simple task.

How would I go about to shorten this up, is there any way I can call the base toolStripButton and tell it to uncheck other buttons when clicked on and check it self?

private void toolStripButton1_Click(object sender, EventArgs e)
{
    if (toolStripButton1.Checked == true)
    {
        toolStripButton2.Checked = false;
        toolStripButton3.Checked = false;
        toolStripButton4.Checked = false;
        toolStripButton5.Checked = false;
        toolStripButton6.Checked = false;
    }
}
private void toolStripButton2_Click(object sender, EventArgs e)
{
    if (toolStripButton2.Checked == true)
    {
        toolStripButton1.Checked = false;
        toolStripButton3.Checked = false;
        toolStripButton4.Checked = false;
        toolStripButton5.Checked = false;
        toolStripButton6.Checked = false;
    }
}

private void toolStripButton3_Click(object sender, EventArgs e)
{
    if (toolStripButton3.Checked == true)
    {
        toolStripButton1.Checked = false;
        toolStripButton2.Checked = false;
        toolStripButton4.Checked = false;
        toolStripButton5.Checked = false;
        toolStripButton6.Checked = false;
    }
}

private void toolStripButton4_Click(object sender, EventArgs e)
{
    if (toolStripButton4.Checked == true)
    {
        toolStripButton2.Checked = false;
        toolStripButton3.Checked = false;
        toolStripButton1.Checked = false;
        toolStripButton5.Checked = false;
        toolStripButton6.Checked = false;
    }

}

private void toolStripButton5_Click(object sender, EventArgs e)
{
    if (toolStripButton5.Checked == true)
    {
        toolStripButton2.Checked = false;
        toolStripButton3.Checked = false;
        toolStripButton1.Checked = false;
        toolStripButton4.Checked = false;
        toolStripButton6.Checked = false;
    }
}

private void toolStripButton6_Click(object sender, EventArgs e)
{
    if (toolStripButton6.Checked == true)
    {
        toolStripButton2.Checked = false;
        toolStripButton3.Checked = false;
        toolStripButton1.Checked = false;
        toolStripButton5.Checked = false;
        toolStripButton4.Checked = false;
    }

}
\$\endgroup\$

migrated from stackoverflow.com Aug 16 '16 at 17:44

This question came from our site for professional and enthusiast programmers.

  • \$\begingroup\$ You do know this is how RadioButton works out of the box. And is how a user would expect a RadioButton to work. \$\endgroup\$ – paparazzo Aug 16 '16 at 22:17
  • 2
    \$\begingroup\$ There are no RadionButtons available in a ToolStrip. \$\endgroup\$ – TaW Aug 17 '16 at 6:44
  • \$\begingroup\$ Gather all the buttons in a list. Write a single event handler and register it with all those buttons. In the handler, foreach thusly: nextButton.Checked(nextButton.Equals(sendingButton)) \$\endgroup\$ – radarbob Aug 18 '16 at 12:15
8
\$\begingroup\$

Yes, this can easily be achieved with a few non-redundant lines.

You can hook up all button clicks to this common MouseDown event:

private void toolStripButton_MouseDown(object sender, MouseEventArgs e)
{
    ToolStripButton button = sender as ToolStripButton;
    if (button == null) throw new InvalidCastException();
    foreach (ToolStripItem item in button.Owner.Items)
    {   
        if (item is ToolStripButton)
        {
            ((ToolStripButton)item).Checked = (item == button);
        }
    }
}

Note that the Click event doesn't allow you to find out which button was clicked as all button clicks map to the ToolStrip itself! Therefore it doesn't lend itself to any sort of common processing that makes use of the sender paramter all events have..

As ever so often MouseDown works much better ;-)

The logic above assumes you want a radiobutton-like behaviour. RadioButtons can only be switched on; to switch them off you need to click another RadioButton in the group. If you want to be able to switch a button on and off directly you can expand it a little..

\$\endgroup\$
  • \$\begingroup\$ If you only had put more of {} there I'd give you a +1 ;-P \$\endgroup\$ – t3chb0t Aug 16 '16 at 18:21
  • \$\begingroup\$ Well, either two or none ;-) \$\endgroup\$ – TaW Aug 16 '16 at 20:15
  • \$\begingroup\$ Agreed on both points. Well, this answer was moved here from SO, where it is quite normal to leave out all sorts of checks without warning and write out only the bare solution to the core issue, not perfeced code.. But since we are here now.. \$\endgroup\$ – TaW Aug 17 '16 at 6:28
  • 1
    \$\begingroup\$ Why make a soft-cast only to throw an InvalidCastException manually when the as cast evaluates to null? Seems simply casting (ToolStripButton)sender would do the same thing with a little less boilerplate. \$\endgroup\$ – Mathieu Guindon Aug 17 '16 at 13:41
  • \$\begingroup\$ Hm, not sure I agree; this is a message to the developer and an invitation to expand it according to one's policy, like sending a custom message, like 'Only hook this up to ToolStripButtons!'.. Simply letting the exception happen can't ever be a good idea, imo. \$\endgroup\$ – TaW Aug 17 '16 at 16:44
2
\$\begingroup\$

Simply collect all the buttons in a list, and then you can use a single event handler for all the check buttons:

private List<ToolStripButton> buttons = new List()
{
    toolStripButton1,
    toolStripButton2,
    toolStripButton3,
    toolStripButton4,
    toolStripButton5,
    toolStripButton6
};

private void toolStripButton_Click(object sender, EventArgs e)
{
    // skip if the current button is not checked
    if (!((ToolStripButton)sender).Checked)
        return;

    // loop through all buttons and uncheck them
    foreach (var btn in buttons)
    {
        // do not uncheck current button
        if (btn != sender)
            btn.Checked = false;
    }
}
\$\endgroup\$
  • \$\begingroup\$ Two answers, both missing {}... conspiracy? ;-) \$\endgroup\$ – t3chb0t Aug 16 '16 at 18:23
  • \$\begingroup\$ Well, I do have some… :P And I think my answer was first, so obviously TaW copied that from me!!! :D ;D \$\endgroup\$ – poke Aug 16 '16 at 18:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy