Refactor that code ... Controls.Find method

Question

OK, I have a code (see below):

    void M1()
    {
        for (int i = 1; i < 10; i++)
        {
            Control[] carr = this.Controls.Find("Number" +
                                                (i - 1).ToString() +
                                                "CheckBox", true);

            if ((carr != null) && (carr.Length > 0))
            {
                CheckBox enableCB = carr[0] as CheckBox;

                enableCB.Checked = i % 2 == 0 ? true : false; // or any other value
            }
        }
    }

I don't like iteration with using Controls.Find method. Can I replace it with something easier ?

PS: all the NumberXCheckBox (X>=0 and X<=8) presented on the form.

LINQ enabled, .net 3.5, Visual studio 2008.

Answer 1

If you are using 3.5 or otherwise have LINQ available you could do the following

for ( int i = 0; i < 9; i++) {
  var control = this.Controls
    .Find(String.Format("Number{0}Checkbox", i))
    .Cast<CheckBox>()
    .FirstOrDefault();
  if ( control != null ) {
    control.Checked = (i % 2) != 0;
  }
}

Answer 2

More linq-y

for (int i = 0; i < 10; i++) {
    var c = (from CheckBox c in this.Controls.Find(String.Format(CultureInfo.InvariantCulture, "Number{0}CheckBox", i-1), true)
             select c).FirstOrDefault();
    if (c != null) {
        c.Checked = i % 2 == 0 ? true : false;
    }
}

Answer 3

I would encourage you to introduce a field in your type to keep references to your checkboxes (an array, a list, a dictionary -- you choose.) This way you'll no longer need to use this non-typed and somewhat ugly find-control-by-key-method.

Anyway, if you're still in .NET 2.0 and prefer to use the Find method, you could simplify a little bit your loop:

for (var i = 0; i <= 8; i++)
{
    var controls = Controls.Find("Number" + i + "CheckBox", true);
    if (controls.Length > 0)
    {
        var checkBox = controls[0] as CheckBox;
        if (checkBox != null)
            checkBox.Checked = i%2 == 0;
    }
}

The latest non-nullity test on checkbox can probably be omitted.

Answer 4

Here's one without linq, but cleaned up the code a little bit.

for (int i = 1; i < 10; i++)
{
    Control[] carr = this.Controls.Find("Number" + (i - 1) + "CheckBox", true);
    if (carr.Length <= 0) continue;
    CheckBox enableCB = carr[0] as CheckBox;
    enableCB.Checked = (i % 2) == 0;
}

[Edit: added the Find(..) code to show why you don't have to check for null]

Here's the frameworks internal code for the Find function. I've added a couple of comments in it

public Control[] Find(string key, bool searchAllChildren)
{
   if (string.IsNullOrEmpty(key))
   {
       throw new ArgumentNullException("key", SR.GetString("FindKeyMayNotBeEmptyOrNull"));
   }
   // Will always return an ArrayList with zero or more elements
   ArrayList list = this.FindInternal(key, searchAllChildren, this, new ArrayList());
   // Will always return an Array of zero or more elements
   Control[] array = new Control[list.Count];
   list.CopyTo(array, 0);
   return array;

}

Answer 5

Instead of searching by name, can you wrap your CheckBox controls in a container that means you can just iterate through the controls in the container?

Answer 6

    void M1()
    {
        for (int i = 1; i < 10; i++)
        {
            var checkBoxes = this.Controls
                .Cast<Control>()
                .Where(c => c.ID.Equals(String.Format("Number{0}CheckBox", i - 1), StringComparison.InvariantCultureIgnoreCase) && c is CheckBox)
                    .ToList();

            if ((checkBoxes != null) && (checkBoxes.Count > 0))
                checkBoxes.ForEach(c => (c as CheckBox).Checked = i % 2 == 0 ? true : false);
        }
    }