The answer of BCdotWEB provides several good points on improvement. Here's my addition to this.
What struck me is the loop. You iterate all the checkboxes every time the value of the numeric up/down changes. In this scenario, when you only have 8 of them, this is not a big matter. But the code can be simplified though.
The solution to avoid the loop is to keep track of the previous value of the numeric up/down and determine if the new value is higher or lower. Based on this, you select the correct checkbox and you only have to set this one checkbox visible/checked or not.
The code will make this much clearer, I'll provide comment in the code to explain. (I assumed that all checkboxes are set to Visible = false
and the numeric up/down has a default value of 0).
//private field to keep track of the previous value
private decimal _previousValue;
private void attEdgeUpDown_ValueChanged(object sender, EventArgs e)
{
//get new value
var edgeValue = attEdgeUpDown.Value;
//check if the new value is higher or lower
var isHigher = edgeValue > _previousValue;
//if the new value is higher than before, use that value
//if the new value is lower, take the value + 1
var checkboxName = "attEdgeBox" + (isHigher ? edgeValue : edgeValue + 1);
var checkboxControl = Controls.Find(checkboxName, true).FirstOrDefault() as CheckBox;
//set the visibility
checkboxControl.Visible = isHigher;
//if the new value is lower, uncheck the checkbox
if (!isHigher)
checkboxControl.Checked = false;
//store the current value in the `_previousValue` field
_previousValue = edgeValue;
}
So for example, you have 0 as current value and you go up 1. This means that isHigher
will be true
and we will use the value of edgeValue
, which is 1. We will take attEdgeBox1
and set it visible, we don't check/uncheck it. At last we store the value 1
in the _previousValue
field.
A reverse example. We have a current value of 7 and we go down 1. isHigher
will be false
and we will use the value of edgeValue + 1
. This means we will take attEdgeBox7
and set it invisible, we also uncheck it. At last, store the value 6 in _previousValue
.
Edit:
Following code allows manual input of a value in the numeric up/down. Since this means that the difference between the new value and the previous value can be more than 1, you cannot evade using a loop. Following code minimizes the loop, meaning you don't have to iterate over all checkboxes.
private void attEdgeUpDown_ValueChanged(object sender, EventArgs e)
{
var edgeValue = attEdgeUpDown.Value;
var isHigher = edgeValue > _previousValue;
var min = isHigher ? _previousValue : edgeValue;
var max = isHigher ? edgeValue : _previousValue;
while (min++ < max)
{
var checkboxName = "attEdgeBox" + min;
var checkboxControl = Controls.Find(checkboxName, true).FirstOrDefault() as CheckBox;
checkboxControl.Visible = isHigher;
if (!isHigher)
checkboxControl.Checked = false;
}
_previousValue = edgeValue;
}