Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Three radio buttons and a text box, grouped in one group box... Just when I select one of those radio button the text box gets enabled, for the other two radio buttons it should be disabled. Here is the code that comes to mind at first, but it looks ugly to me becuase I have copy pasted the same AllowMissingData() method to Change evnt of each radio button. I was wondering if there is a better way of writing it:

private void RequiredRadioButton_CheckedChanged(object sender, EventArgs e)
{
        AllowMissingData();
}

private void AllowBlankRadioButton_CheckedChanged(object sender, EventArgs e)
{
        AllowMissingData();
}

private void SuppressRadioButton_CheckedChanged(object sender, EventArgs e)
{
        AllowMissingData();
}

private void AllowMissingData()
{
    if (AllowBlankRadioButton.Checked)
    {
        MissingDataValueTextBox.Enabled = true;
    }
    else
    {
        MissingDataValueTextBox.Text = System.String.Empty;
        MissingDataValueTextBox.Enabled = false;
    }
}
share|improve this question

2 Answers

up vote 3 down vote accepted

Did you realize that multiple RadioButtons can all point to the same event handler?

(simply use the Visual Studio Properties Editor in the designer to assign the same handler. Alternatively, you could apply += AnyRadioButton_CheckedChanged to each of the RadioButtons' CheckedChanged events.)

private void AnyRadioButton_CheckedChanged(object sender, EventArgs e)
{
    if (allowBlankRadioButton.Checked)
    {
        missingDataValueTextBox.Enabled = true;
    }
    else
    {
        missingDataValueTextBox.Enabled = false;
        missingDataValueTextBox.Text = string.Empty;
    }
}

I actually find the if-else very readable, so I resisted the urge to incorporate Jeff's concise answer. In my opinion, the more elaborate version is clearer about what it does.

I renamed all your controls to follow the camelCase naming convention (because PascalCase should be reserved for type names, etc).

share|improve this answer

This cleans it up a little. Removes the if statement at least:

private void AllowMissingData()
{
    MissingDataValueTextBox.Enabled = AllowBlankRadioButton.Checked;
    MissingDataValueTextBox.Text = AllowBlankRadioButton.Checked ? MissingDataValueTextBox.Text : System.String.Empty;
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.