HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Flags on steroids

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
flagssteroidsstackoverflow

Problem

What is so bad about [Flags] enum? It does not support IEnumerable anyhow, so to get one we need to use syntax like (see):

new HashSet(new []{ 
    WhatToInclude.Cleaners, 
    WhatToInclude.Managers}


Here is the helper class Flag to discuss. We can use it this way:

class Color : Flag
{
    public static readonly Color Red = new Color();
    public static readonly Color Green = new Color();
    public static readonly Color Yellow = new Color();
}


Manipulation and testing:

Color c = Color.None;
c += Color.Yellow + Color.Red;
c -= Color.Yellow + Color.Green;

// it implements IEnumerable
bool t1 = c.Contains(Color.Red) // true
bool t2 = c.Contains(Color.None) // always false


Refactoring of enumeration to classes is half way done here already :)

Library code:

abstract class Flag : IEnumerable 
    where T : Flag, new()
{
    public static T None = new T() { Flags = new T[0] };

    protected Flag()
    {
        Flags = new[] { (T)this };
    }

    IEnumerable Flags { get; set; }
    public IEnumerator GetEnumerator() => Flags.GetEnumerator();
    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

    public static T operator+(Flag left, Flag right)
    {
        return new T()
        {
            Flags = left.Concat(right).Distinct().ToArray()
        };
    }

    public static T operator-(Flag left, Flag right)
    {
        return new T()
        {
            Flags = left.Except(right).ToArray()
        };
    }
}


Extra example

class FamilyMember : Flag
{
    public static readonly FamilyMember Me = new FamilyMember() { Name = "Dmitry" };
    public static readonly FamilyMember Wife = new FamilyMember() { Name = "Julia" };
    public static readonly FamilyMember Cat = new FamilyMember() { Name = "Willy" };
    public static readonly FamilyMember All = Me + Wife + Cat;

    public string Name { get; private set; }
}


We can use it in a very simple way:

```
foreach (var fm in FamilyMember.All - Fa

Solution

I'm not going to say I'd use this, nor that I wouldn't.

I think it's a clever1 use of generics, and turns C# enums into something else that could possibly be called JavaIshEnum; being an actual class, you significantly blur the line between classes and enums, but you also effectively work around the annoying absence of a generic type constraint in the C# compiler, that can constraint enum types.

Or do you?

public class Kaboom : Flag
{
    ...
}


This derived type can be legally constructed... and shouldn't be allowed to. Of course, compile-time check is going to be hard (impossible?) - but your code is missing a run-time check.

I'd make the base constructor throw and forbid the creation of a non-enum T:

protected Flag()
{
    if (!typeof(T).IsEnum)
    {
        throw new ArgumentException("Non-enum type is not a valid type argument.");
    }
    Flags = new[] { (T)this };
}


I don't like this:

IEnumerable Flags { get; set; }
public IEnumerator GetEnumerator() => Flags.GetEnumerator();
IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();


You're only specifying access modifiers when you're overriding the default. I would have liked to see this:

private IEnumerable Flags { get; set; }
public IEnumerator GetEnumerator() => Flags.GetEnumerator();
private IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();


And then raise the question, why does Flags need to be an auto-property? If it's private, wouldn't just the backing field be enough? I don't get the point of private properties.. I just don't.

I also don't like that you have private and public members intertwined. Regroup them.

1 is that a good thing?

Code Snippets

public class Kaboom : Flag<SomeClass>
{
    ...
}
protected Flag()
{
    if (!typeof(T).IsEnum)
    {
        throw new ArgumentException("Non-enum type is not a valid type argument.");
    }
    Flags = new[] { (T)this };
}
IEnumerable<T> Flags { get; set; }
public IEnumerator<T> GetEnumerator() => Flags.GetEnumerator();
IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
private IEnumerable<T> Flags { get; set; }
public IEnumerator<T> GetEnumerator() => Flags.GetEnumerator();
private IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

Context

StackExchange Code Review Q#117845, answer score: 15

Revisions (0)

No revisions yet.