10
\$\begingroup\$

I'm clearing all the controls on a windows form with the following

form.Controls.Cast<dynamic>().ToList().ForEach(c =>
{
      switch (c)
      {
          case CheckBox t when c is CheckBox:
              c.Checked = false;
              break;
          case System.Windows.Forms.ComboBox t when c is System.Windows.Forms.ComboBox:
              c.Items.Clear();
              break;
          case CheckEdit t when c is CheckEdit:
              c.Checked = false;
              break;
          default:
              c.Text = "";
              break;
      }
});

I have to use dynamic because if I dont, I wont get the options for Checked and I also wont be able to clear any items in a combobox or list for that matter.

Is there a neater way of achieving this? I'm using c# 7.3 which allows me to use the when keyword, I wasn't able to find how to do this with anything under 7.0, If there is something for under 7.0, what would it be?

\$\endgroup\$
  • \$\begingroup\$ Why are you seeking a way how to do it in C# 7.0 if you can use the latest 7.3? \$\endgroup\$ – t3chb0t Aug 2 '18 at 10:33
  • \$\begingroup\$ ` If there is something for under 7.0, what would it be?` \$\endgroup\$ – AndrewE Aug 2 '18 at 10:34
  • \$\begingroup\$ Still, why? Why would someone want to use something older if he can do it much easier with the latest version? \$\endgroup\$ – t3chb0t Aug 2 '18 at 10:35
  • \$\begingroup\$ work is still using 6.0 \$\endgroup\$ – AndrewE Aug 2 '18 at 10:35
  • \$\begingroup\$ If so then your question is off-topic because it's unimplemented code. \$\endgroup\$ – t3chb0t Aug 2 '18 at 10:36
12
\$\begingroup\$

I think you're rather over-complicated this for yourself! A much better solution is to use the pattern matched results (which you are already creating (t)) (this requires C# 7.0 still, I believe).

foreach (Control c in form.Controls)
{
    switch (c)
    {
        case CheckBox cb:
            cb.Checked = false;
            break;
        case ComboBox cb:
            cb.Items.Clear();
            break;
        case CheckEdit ce:
            ce.Checked = false;
            break;
        default:
            c.Text = "";
            break;
    }
}
  • No dynamic here: Text is already a member of Control.

  • Foreach loops are easier to read than .Foreach (in my opinion, at least; even if they are a bit funny underneath)

  • Your original ToList was redundant: no need to cache a result you are going to stream immediately in single-threaded logic

  • I've renamed t to something slightly less terrible; since they are very local variables indeed, I personally wouldn't worry to much about the names.

  • Note that this code will also clear the text on labels, and buttons, and everything else... so it seems an odd thing to provide as a form-wide operation.

\$\endgroup\$
  • \$\begingroup\$ That does indeed look much nicer than what I have above even if I was using Linq. You're right your code does indeed still require 7.0 because its using pattern matching, I could be wrong, will need to investigate it. As for clearing everything, indeed this is the point, as part of the function, for an extension helper, our developers will have the option of what control(s) they want to clear, by control type or name and what controls they dont want to clear by control type of name, this will save them writing a method to just having one line. \$\endgroup\$ – AndrewE Aug 2 '18 at 8:23
  • \$\begingroup\$ There is no better solution than this one ;-) \$\endgroup\$ – t3chb0t Aug 2 '18 at 15:12
2
\$\begingroup\$

I believe you should use recursive method to check all controls, even those nested in grouping controls like panels:

 public void ClearControls(Control parent)
        {
            foreach (Control c in parent.Controls)
            {
                switch (c.GetType().ToString())
                {
                    case "CheckBox":
                        (c as CheckBox).Checked = false;
                        break;
                    case "ComboBox":
                        (c as ComboBox).Items.Clear();
                        break;
                    case "CheckEdit":
                        (c as CheckEdit).Checked = false;
                        break;
                    default:
                        c.Text = "";
                        break;
                }

                ClearControls(c);
            }
        }
\$\endgroup\$
  • \$\begingroup\$ Consider checking against at least the type name instead of magic strings, the casts should ideally have clear checks to confirm they worked, and the var aa seems like it might not be needed? \$\endgroup\$ – Dave Aug 2 '18 at 11:59
  • \$\begingroup\$ You are right about the var aa i removed it already. Forgot to delete it before publishing thanks for let me know. Referring to 'magic string' I cannot use Name since switch is expecting constant value with version Framework 4.6.1 on which I tested out this solution. \$\endgroup\$ – Duz Aug 2 '18 at 12:04
  • 2
    \$\begingroup\$ This is actually worse than the OP's solution. All those hardcoded magic-strings and casting with as :-o \$\endgroup\$ – t3chb0t Aug 2 '18 at 15:10
  • \$\begingroup\$ But it works under 7.0, Dont get me wrong I can see VisualMelon's solution is better than this but problem is he needs C# v 7.0+ and this is alternative version for somebody who needs it under 7.0. \$\endgroup\$ – Duz Aug 7 '18 at 7:27

Your Answer

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

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