1

Initially I had the following:

[Flags]
public enum QueryFlag
{
    None = 0x00,
    CustomerID = 0x01,
    SalesPerson = 0x02,
    OrderDate = 0x04
}

As check boxes are checked/unchecked, I would add/remove flags from:

QueryFlag qflag;

My idea - when the user clicks the Search button, I would iterate the actual flags set in qflag to modify a .Where clause in my LINQ to Sql. However, Enum.GetValues(qflag.GetType()) returns all the values of the QueryFlag itself. Not helpful.

My solution:

class myForm : Form
{
    List<QueryFlag> qflag = new List<QueryFlag>();

    private void chkOrderDate_CheckedChanged(object sender, EventArgs e)
    {
        if (chkOrderDate.Checked && !qflags.Contains(QueryFlag.OrderDate))
            qflags.Add(QueryFlag.OrderDate);
        else
            qflags.Remove(QueryFlag.OrderDate);
    }

    private void cmdSearch_Click(object sender, EventArgs e)
    {
        if (qflags.Count == 0)
        {
            rtfLog.AppendText("\nNo search criteria selected.");
            return;
        }

        foreach (QueryFlag flag in qflag)
        {
            rtfLog.AppendText(string.Format("\nSearching {0}", flag.ToString()));

            // add switch for flag value
        }
    }
}

public enum QueryFlag
{
    CustomerID,
    SalesPerson,
    OrderDate
}

I have 3 check boxes and this works without any issues to this point. But I am wondering if there is a better way to perform this iteration.

IAbstract
  • 19,551
  • 15
  • 98
  • 146

2 Answers2

1

The way you had it originally was correct; you just got messed up by the Enum.GetValues method. This method returns every defined value for a particular enum type, yes. So what you do with this is check each defined value against your particular enum value to see whether the defined value is set within your value.

That is, you should do this:

private void chkOrderDate_CheckedChanged(object sender, EventArgs e)
{
    if (chkOrderDate.Checked)
    {
        qFlag |= QueryFlag.OrderDate;
    }
    else
    {
        qFlag &= (~QueryFlag.OrderDate);
    }
}

...and likewise for your other CheckBoxes. Then when you want to enumerate what flags you have set:

static IEnumerable<QueryFlag> GetFlags(QueryFlag flags)
{
    foreach (QueryFlag flag in Enum.GetValues(typeof(QueryFlag)))
    {
        // Presumably you don't want to include None.
        if (flag == QueryFlag.None)
        {
            continue;
        }

        if ((flags & flag) == flag)
        {
            yield return flag;
        }
    }
}

In fact, you could even abstract the above into a handy extension method for all enum types:

public static class FlagsHelper
{
    // This is not exactly perfect, as it allows you to call GetFlags on any
    // struct type, which will throw an exception at runtime if the type isn't
    // an enum.
    public static IEnumerable<TEnum> GetFlags<TEnum>(this TEnum flags)
        where TEnum : struct
    {
        // Unfortunately this boxing/unboxing is the easiest way
        // to do this due to C#'s lack of a where T : enum constraint
        // (there are ways around this, but they involve some
        // frustrating code).
        int flagsValue = (int)(object)flags;

        foreach (int flag in Enum.GetValues(typeof(TEnum)))
        {
            if ((flagsValue & flag) == flag)
            {
                // Once again: an unfortunate boxing/unboxing
                // due to the lack of a where T : enum constraint.
                yield return (TEnum)(object)flag;
            }
        }
    }
}

So your cmdSearch_Click handler could look like this:

private void cmdSearch_Click(object sender, EventArgs e)
{
    if (qFlag == QueryFlags.None)
    {
        rtfLog.AppendText("\nNo search criteria selected.");
        return;
    }

    foreach (QueryFlag flag in qFlag.GetFlags())
    {
        rtfLog.AppendText(string.Format("\nSearching {0}", flag.ToString()));

        // add switch for flag value
    }
}
Dan Tao
  • 125,917
  • 54
  • 300
  • 447
  • This would not allow you to unset bits. The original properly tested the `Checked` property. – Ben Voigt Sep 19 '10 at 17:25
  • @Ben: Good catch; that was just a dumb oversight on my part. I will update the answer. – Dan Tao Sep 19 '10 at 17:26
  • @Dan: thanks...don't know why I didn't think about comparing the flags set in `qflag` to the enumeration values. Makes sense. I don't think I want to go the extension method route - the boxing/unboxing is extremely unappealing for this project; however, I will keep it in mind should the need arise. – IAbstract Sep 19 '10 at 22:07
  • @Dan - To remove an item of the `enum`, you are using `qFlag &= (~QueryFlag.OrderDate);` while I'm using `flags=flags ^ QueryFlag.OrderDate;`. Any difference? Or I made a mistake? Thanks. – Cheng Chen Sep 20 '10 at 02:06
  • @Danny: No, you didn't make a mistake! Either way works, actually: `qFlag &= (~QueryFlag.Order)` (which is less idiomatic, I believe) essentially ANDs each bit in `qFlag` with 1 (*except for* the `QueryFlag.Order` bit) whereas `qFlag ^= QueryFlag.Order` XORs each bit with 0 (except for that bit). As you can see, the result is the same either way. – Dan Tao Sep 20 '10 at 02:58
  • @Dan Tao - Yes I made! See [this question](http://stackoverflow.com/questions/3748516/how-can-i-know-items-is-in-the-enum) I asked just now. – Cheng Chen Sep 20 '10 at 03:23
  • XOR only turns the bit off if it was previously on, while AND NOT always turns it off. – Ben Voigt Sep 20 '10 at 06:05
  • @Danny, @Ben: You're right; setting `flags ^= QueryFlag.OrderDate` actually *toggles* the `OrderDate` bit rather than turns it indiscriminately off. I suppose when you read others' suggestions to use this method, they are generally assuming that you already know for a fact the bit is set. So `flags &= ~QueryFlag.OrderDate` is more accurately the same as `if ((flags & QueryFlag.OrderDate) == QueryFlag.OrderDate) { flags ^= QueryFlag.OrderDate; }` – Dan Tao Sep 20 '10 at 06:57
0

The short, cheesy, and inefficient way:

qflag.ToString().Split('|')
Ben Voigt
  • 277,958
  • 43
  • 419
  • 720
  • Yeah, but then he's got to parse the strings back to enums. But you're right: even this will work. – Dan Tao Sep 19 '10 at 17:24
  • Parsing the strings back to enums might not be forward progress, at this point I'd probably reach for a `Dictionary`. With a call to `string.Trim()` thrown in there somewhere. – Ben Voigt Sep 19 '10 at 17:27
  • Yes, but really at that point haven't you arrived at a solution about as bloated as what the OP already had (with a `List`)? I think he really was about 90% of the way there already with his `[Flags]`-based approach; he just got thrown off when it came time to *enumerate* a set of flags. – Dan Tao Sep 19 '10 at 17:31
  • Bloated in terms of runtime inefficiency, sure. But in terms of code reuse, it's actually pretty good. Especially if `ControlCollection::Find` is used, then the designer can be used to wire up the list. – Ben Voigt Sep 19 '10 at 17:46
  • Hhmmm...interesting. @Ben is correct; I got thrown off expecting specific results. When I got unexpected results, I resorted to another solution. Frankly, if it came down to choosing my implementation or the `.Split('|')`, I would choose mine. :) – IAbstract Sep 19 '10 at 22:18