Check against groups of constants [closed]

Question

In my .NET Framework app (stuck at C# 7.3), I use many if checks against group of constants in so many places (maintained more than 10 years).

Existing:

public static class Constants
{
    public const int Source1 = 111;
    public const int Source2 = 123;
    public const int Source3 = 135;
    // many more
    public const int Source999 = 1234;
}

// in some place
if(source == Constants.Source1 // belongs to group A
|| source == Constants.Source2 // belongs to group A
|| source == Constants.Source3 // belongs to group A
|| source == Constants.Source4 // belongs to group A & B
|| source == Constants.Source5 // belongs to group A & B
|| source == Constants.Source6) // belongs to group A & B
{ /** do something **/ }

// in other place
if(source == Constants.Source9 // belongs to group C
|| source == Constants.Source10 // belongs to group C
|| source == Constants.Source11) // belongs to group C
{ /** do something **/ }

// combination of groups B, C & D
if(source == Constants.Source4 // belongs to group A & B
|| source == Constants.Source5 // belongs to group A & B
|| source == Constants.Source6 // belongs to group A & B
|| source == Constants.Source7 // belongs to group B
|| source == Constants.Source8 // belongs to group B
|| source == Constants.Source9 // belongs to group C
|| source == Constants.Source10 // belongs to group C
|| source == Constants.Source11 // belongs to group C
|| source == Constants.Source12 // belongs to group D 
|| source == Constants.Source13 // belongs to group D
|| source == Constants.Source14 // belongs to group D & E
|| source == Constants.Source15) // belongs to group D & E
{ /** do something **/ }

// groups B, C & D with exception of A & E
if(source == Constants.Source7 // belongs to group B
|| source == Constants.Source8 // belongs to group B
|| source == Constants.Source9 // belongs to group C
|| source == Constants.Source10 // belongs to group C
|| source == Constants.Source11 // belongs to group C
|| source == Constants.Source12 // belongs to group D 
|| source == Constants.Source13) // belongs to group D
{ /** do something **/ }

The problem:

Readability and maintainability.

I always change many conditions when introducing new source. After years, the need of grouping them becoming clear.

The change:

Instead of checking against group of constants, it should check against group of groups of constants.

So I start defining the groups:

public static class SourceGroups
{
    public static int[] A => new[] {
            Constants.Source1,
            Constants.Source2,
            Constants.Source3,
            Constants.Source4,
            Constants.Source5,
            Constants.Source6
    };

    public static int[] B => new[] {
            Constants.Source4,
            Constants.Source5,
            Constants.Source6,
            Constants.Source7,
            Constants.Source8
    };

    // many more
}

// most usages becomes very simple
if (SourceGroups.A.Contains(source))
{ /** do something **/ }
    
// as well as combination of groups
if (SourceGroups.B.Contains(source)
 || SourceGroups.C.Contains(source)
 || SourceGroups.D.Contains(source))
{ /** do something **/ }

// combination with exception
foreach(var item in items)
{
  var source = item.Source;

  // it sacrifice performance here due to preparing the correct list before checking
  var eligibleForOperation1 = SourceGroups.B
                      .Concat(SourceGroups.C)
                      .Concat(SourceGroups.D)
                      .Except(SourceGroups.A)
                      .Except(SourceGroups.E);

  if (eligibleForOperation1.Contains(item.Source))
  {
    // operation 1
  }

  // and sacrifice performance here
  var eligibleForOperation2 = SourceGroups.F
                      .Concat(SourceGroups.G)
                      .Except(SourceGroups.H);

  if (eligibleForOperation2.Contains(item.Source))
  {
    // operation 2
  }
}

Performance sacrifice in the last case. But, I guess, It's still okay-ish.

I might miss something so I appreciate if you guys have any feedback.

Answer 1

I suggest a different model where you declare each constant only once and specify the groups it belongs to. For this we declare the following struct:

public readonly struct Source
{
    public readonly int Value;
    public readonly bool A;
    public readonly bool B;
    public readonly bool C;
    public readonly bool D;
    public readonly bool E;

    public Source(int value, bool a = false, bool b = false, bool c = false, bool d = false, bool e = false)
    {
        Value = value;
        A = a;
        B = b;
        C = c;
        D = d;
        E = e;
    }
}

The constructor parameters a to e are optional and default to false. We can then easily declare the sources by specifying only the groups they belong to by using named parameters:

public static class Constants
{
    public static readonly Source Source1 = new Source(111, a: true);
    public static readonly Source Source2 = new Source(123, a: true);
    public static readonly Source Source3 = new Source(135, a: true);
    public static readonly Source Source4 = new Source(147, a: true, b: true);
    public static readonly Source Source5 = new Source(159, a: true, b: true);
    // ...

    public static readonly Dictionary<int, Source> Sources = new Dictionary<int, Source> {
        [Source1.Value] = Source1,
        [Source2.Value] = Source2,
        [Source3.Value] = Source3,
        [Source4.Value] = Source4,
        [Source5.Value] = Source5,
        // ...
    };
}

You would get a constant with int c = Constants.Source5.Value;.

And then do checks like this by querying the dictionary once (note that a dictionary lookup O(1) is faster than searching an array O(n)):

int sourceNumber = 135;
var source = Constants.Sources[sourceNumber];
if (source.B || source.C || source.D) {
    // Do something
}

If you are not sure whether a number is a valid source, you can test

if (Constants.Sources.TryGetValue(sourceNumber, out Source source) &&
    (source.B || source.C || source.D)) {

    // Do something
}