2
\$\begingroup\$

I'm looking for a way to get a subset of a given enum to avoid repetition and use same enum type while comparing.

I found this question Enum subset or subgroup in C# and like the idea of using static class. Whoever, It doesn't allow to iterate over all elements in the class. So I added GetValues method.

Edit:

Here is my full code:

    public partial class Devices : Form
        {
            public Devices()
            {
                InitializeComponent();

                // Here, I need to iterate over StatusEnum items
                foreach (var item in Enum.GetValues(typeof(StatusEnum)))
                {
                    checkedListBoxParent.Items.Add(item);
                }

                // and iterate over ValidStatusEnum items
                foreach (var item in ValidStatusEnum.GetValues())
                {
                    checkedListBoxChild.Items.Add(item);
                }
            }

            private void On_MouseUp(object sender, MouseEventArgs e)
            {
                // Here, I need to compare StatusEnum and ValidStatusEnum
                if (Equals(checkedListBoxParent.CheckedItems.Cast<StatusEnum>().ToList(),
                    checkedListBoxChild.CheckedItems.Cast<StatusEnum>().ToList()))
                {
                    MessageBox.Show("Parent and child are similar");
                }
            }

            private bool Equals(List<StatusEnum> list1, List<StatusEnum> list2)
            {
                if (list1.Count != list2.Count)
                    return false;
                for (int i = 0; i < list1.Count; i++)
                {
                    if (list1[i] != list2[i])
                        return false;
                }
                return true;
            }

            private bool Equals(StatusEnum status, StatusEnum validStatus)
            {
                return status == validStatus;
            }
        }

        public enum StatusEnum { Unknown, Known, NotAvailable, Available };

        internal static class ValidStatusEnum
        {
            public const StatusEnum Known = StatusEnum.Known;
            public const StatusEnum Available = StatusEnum.Available;

            public static StatusEnum[] GetValues()
            {
                return new StatusEnum[] { Known, Available };
            }
        }
}

So basically, in my winforms application, I have two CheckedListBoxes (Parent and Child) populated from StatusEnum Enum and ValidStatusEnum static class. So, I need to iterate over enums to populate Checkboxes and I need to compare StatusEnum and ValidStatusEnum to display a message to user in case I have the same selection in both checkboxes.

Any thoughts about the design and the code?

\$\endgroup\$
  • 3
    \$\begingroup\$ You mention iterating over the values in order to compare. Could you give an example of this? It looks, to me, like a refactoring might in order. \$\endgroup\$ – tinstaafl Jun 7 '17 at 17:04
  • 1
    \$\begingroup\$ I agree with @tinstaafl. This feels like a weird thing to do, so more context about why you want to do it would be helpful. \$\endgroup\$ – 200_success Jun 7 '17 at 17:36
  • \$\begingroup\$ @tinstaafl Image a method that has as parameter a StatusEnum and will find out if it is part of ValidStatusEnum \$\endgroup\$ – Mhd Jun 7 '17 at 18:13
  • \$\begingroup\$ That doesn't provide an example of usage. An example means actual code that shows how your code would be used. \$\endgroup\$ – tinstaafl Jun 7 '17 at 18:52
  • \$\begingroup\$ I rolled back the edit that introduced illegal code, feel free to add a real example with actual working code. \$\endgroup\$ – Mathieu Guindon Jun 7 '17 at 20:21
4
\$\begingroup\$

This answer assumes there's no way the different "groups" would actually belong in different enum types. Otherwise the only sensible answer is "if you have different groups of related values, put them in different enum types". Status is a fairly broad name (enum types shouldn't have "Enum" in their names) that doesn't really says anything - if you have an Availability group with NotAvailable and Available possible values, then have an Availability enum with these values.


That's [very] little more than a hard-coded Enum.GetValues helper...

Define your enum vertically:

public enum Status
{
    Unknown,
    Known,
    NotAvailable,
    Available,
}

Then it's much easier to add attributes, e.g. a [Description]:

public enum Status
{
    [Description("Device is unknown.")]
    Unknown,
    [Description("Device is known.")]
    Known,
    [Description("Device is not available.")]
    NotAvailable,
    [Description("Device is available.")]
    Available,
}

Or whatever. If you have enum values that semantically related and need to regroup them, you can use flags for that. Use powers of 2 to specify explicit underlying values for each enum; you can use bit-shifting to simplify that:

[Flags]
public enum Status
{
    Unknown = 1 << 0,
    Known = 1 << 1,
    NotAvailable = 1 << 2,
    Available = 1 << 3,
}

And then you can define the "groupings" as you please:

[Flags]
public enum Status
{
    Unknown = 1 << 0 | Knowledge | Negative,
    Known = 1 << 1 | Knowledge | Positive,
    NotAvailable = 1 << 2 | Availability | Negative,
    Available = 1 << 3 | Availability | Positive,
    Knowledge = 1 << 20,
    Availability = 1 << 21,
    Positive = 1 << 22,
    Negative = 1 << 23,
}

And now when you're faced with a Status value, you can do this:

var isPositive = value.HasFlag(Status.Positive);

And you can get all Positive enum values rather easily:

var positiveStatusCodes = Enum.GetValues(typeof(Status))
                              .Cast<Status>()
                              .Where(status => status.HasFlag(Status.Positive))
                              .ToArray();

That way when you need to add enum values, just give them the proper flags and your "get all positive status codes" code doesn't need to change at all.

\$\endgroup\$
  • \$\begingroup\$ Let me be the first to say that... there is a performance issue with HasFlag ;-] \$\endgroup\$ – t3chb0t Jun 7 '17 at 18:18
  • 1
    \$\begingroup\$ @t3chb0t sure. Use bitshifting then. If you're not working in a loop there's no reason to fear the performance hit of a single instruction. Readability wins IMO. \$\endgroup\$ – Mathieu Guindon Jun 7 '17 at 18:19
  • 3
    \$\begingroup\$ Needless to say. I just wanted to be the first this time because nearly every answer containing HasFlag has also such comment. \$\endgroup\$ – t3chb0t Jun 7 '17 at 18:24
  • \$\begingroup\$ FWIW I'm asking on Software Engineering if this is a legit approach... I have doubts. \$\endgroup\$ – Mathieu Guindon Jun 7 '17 at 20:06
3
\$\begingroup\$
public enum StatusEnum { Unknown, Known, NotAvailable, Available };

This enum has two issues:

  • We don't add the Enum suffix to enum names, it should be just Status.
  • The value Known does not make sense. To be able to say if something is (not)available you have to know it, haven't you? So isn't it automatically known?

internal static class ValidStatusEnum

The issues with this fake enum is that it's a class. You should not name classes as if they were enums. Also valid status for what? Usually any enum value greater then Unknown = 0 could be valid.


I find this design does more harm then good. If you want to have enum value groups for certain purposes then instead of repeating them as properties use enumerable properties to return the values:

static class StatusGroups
{
    public static IEnumerable<Status> Availibility 
    {
        get
        {
            yield return Status.NotAvailable;
            yield return Status.Available;
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Meh. Quickly becomes a maintenance nightmare IMO. Less so than OP's code, but none of that is actually needed if you use [Flags]. \$\endgroup\$ – Mathieu Guindon Jun 7 '17 at 18:11
  • \$\begingroup\$ @Mat'sMug either this or a device can be not-available and available at the same time which isn't really better :-] \$\endgroup\$ – t3chb0t Jun 7 '17 at 18:13
  • \$\begingroup\$ That's assuming input validation is not a thing... \$\endgroup\$ – Mathieu Guindon Jun 7 '17 at 18:14
  • \$\begingroup\$ @Mat'sMug Input validation? I've never heard of that. My input is always valid ;-P I wouldn't use flags for something that's not really flaggable. \$\endgroup\$ – t3chb0t Jun 7 '17 at 18:17
  • \$\begingroup\$ So this is abuse then? \$\endgroup\$ – Mathieu Guindon Jun 7 '17 at 18:18

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.