How could I simplify this long chain of conditional statements?

0

I have the following enum

[Flags]
internal enum DataSectionFlags : uint
{
    TypeReg = 0x0,
    TypeDsect = 0x01,
    TypeNoLoad = 0x02,
    TypeGroup = 0x04,
    TypeNoPadded = 0x08,
    TypeCopy = 0x010,

    ContentCode = 0x020,
    ContentInitializedData = 0x040,
    ContentUninitializedData = 0x080,

    LinkOther = 0x0100,
    LinkInfo = 0x0200,

    TypeOver = 0x0400,

    LinkRemove = 0x0800,
    LinkComDat = 0x01000,

    NoDeferSpecExceptions = 0x04000,

    RelativeGP = 0x08000,

    MemPurgeable = 0x020000,

    Memory16Bit = 0x020000,
    MemoryLocked = 0x040000,
    MemoryPreload = 0x080000,

    Align1Bytes = 0x0100000,
    Align2Bytes = 0x0200000,
    Align4Bytes = 0x0300000,
    Align8Bytes = 0x0400000,
    Align16Bytes = 0x0500000,
    Align32Bytes = 0x0600000,
    Align64Bytes = 0x0700000,
    Align128Bytes = 0x0800000,
    Align256Bytes = 0x0900000,
    Align512Bytes = 0x0A00000,
    Align1024Bytes = 0x0B00000,
    Align2048Bytes = 0x0C00000,
    Align4096Bytes = 0x0D00000,
    Align8192Bytes = 0x0E00000,

    LinkExtendedRelocationOverflow = 0x01000000,

    MemoryDiscardable = 0x02000000,
    MemoryNotCached = 0x04000000,
    MemoryNotPaged = 0x08000000,
    MemoryShared = 0x10000000,
    MemoryExecute = 0x20000000,
    MemoryRead = 0x40000000,
    MemoryWrite = 0x80000000
}

I am casting a uint variable with this enum like so

var testVariable = (DataSectionFlags) 1610612768;

I have a method that processes the above variable like this

private static uint GetSectionProtection(DataSectionFlags characteristics)
{
    uint result = 0;

    if (characteristics.HasFlag(DataSectionFlags.MemoryNotCached))
    {
        // PageNoCache

        result |= 0x200;
    }

    if (characteristics.HasFlag(DataSectionFlags.MemoryExecute))
    {
        if (characteristics.HasFlag(DataSectionFlags.MemoryRead))
        {
            if (characteristics.HasFlag(DataSectionFlags.MemoryWrite))
            {
                // PageExecuteReadWrite

                result |= 0x40;
            }

            else
            { 
                // PageExecuteRead

                result |= 0x20;
            }

        }

        else if (characteristics.HasFlag(DataSectionFlags.MemoryWrite))
        {
            // PageExecuteWriteCopy

            result |= 0x80;
        }

        else
        {
            // PageExecute

            result |= 0x10;
        }
    }

    else if (characteristics.HasFlag(DataSectionFlags.MemoryRead))
    {
        if (characteristics.HasFlag(DataSectionFlags.MemoryWrite))
        {
            // PageReadWrite

            result |= 0x04;
        }

        else
        {
            // PageReadOnly

            result |= 0x02;
        }               
    }

    else if (characteristics.HasFlag(DataSectionFlags.MemoryWrite))
    {
        // PageWriteCopy

        result |= 0x08;
    }

    else
    {
        // PageNoAccess

        result |= 0x01;
    }

    return result;
}

I'm attempting to simplify the long chain of conditional statements inside this method but am having trouble doing so.

What would be the simplest way to write the conditional statements inside the method whilst still maintaining their functionality?

c#
performance
if-statement
asked on Stack Overflow Nov 21, 2018 by (unknown user)

2 Answers

4

I suggest a lookup dictionary like:

var sectionProtection = new Dictionary<DataSectionFlags, uint>
{
    [DataSectionFlags.TypeReg ] = 1,
    [DataSectionFlags.TypeDsect ] = 2,
    ...
    [DataSectionFlags.MemoryExecute | DataSectionFlags.MemoryRead | DataSectionFlags.MemoryWrite] = 0x40,
    ...
};

Note you'll need a separate entry for every combination of flags. Given such, you can then replace your function with the statement

var testVariable = sectionProtection[(DataSectionFlags) 1610612768];

or, if not every combination is defined:

if (sectionProtection.TryGetValue((DataSectionFlags) 1610612768, out testVariable ))

I consider this not only simpler to understand, faster to run, but also more correct. It is too easy to miss a combination, to have the same combination return different values, or to include the same combination twice when creating a list of if ... else if ... else if ... statements. If you miss a combination in a lookup dictionary you get an exception (or TryGetValue returns false). If you add the same combination to a dictionary twice you get an error.

answered on Stack Overflow Nov 21, 2018 by Dour High Arch • edited Nov 26, 2018 by Dour High Arch
1

This is the best that I could come up with at short notice:

private static uint GetSectionProtection(DataSectionFlags characteristics)
{
    uint result = 0;

    if (characteristics.HasFlag(DataSectionFlags.MemoryNotCached))
    {
        // PageNoCache
        result |= 0x200;
    }

    var ladder = new KeyValuePair<DataSectionFlags[], uint>[]
    {
        new KeyValuePair<DataSectionFlags[], uint>(new [] { DataSectionFlags.MemoryExecute, DataSectionFlags.MemoryRead, DataSectionFlags.MemoryWrite, }, 0x40),
        new KeyValuePair<DataSectionFlags[], uint>(new [] { DataSectionFlags.MemoryExecute, DataSectionFlags.MemoryRead, }, 0x20),
        new KeyValuePair<DataSectionFlags[], uint>(new [] { DataSectionFlags.MemoryExecute, DataSectionFlags.MemoryWrite, }, 0x80),
        new KeyValuePair<DataSectionFlags[], uint>(new [] { DataSectionFlags.MemoryExecute, }, 0x10),
        new KeyValuePair<DataSectionFlags[], uint>(new [] { DataSectionFlags.MemoryRead, DataSectionFlags.MemoryWrite, }, 0x04),
        new KeyValuePair<DataSectionFlags[], uint>(new [] { DataSectionFlags.MemoryRead, }, 0x02),
        new KeyValuePair<DataSectionFlags[], uint>(new [] { DataSectionFlags.MemoryWrite, }, 0x08),
        new KeyValuePair<DataSectionFlags[], uint>(new DataSectionFlags[] { }, 0x01),
    };

    result |= ladder.Where(x => x.Key.All(y => characteristics.HasFlag(y))).First().Value;

    return result;
}

A possibly more readable version:

private static uint GetSectionProtection(DataSectionFlags characteristics)
{
    uint result = 0;

    if (characteristics.HasFlag(DataSectionFlags.MemoryNotCached))
    {
        // PageNoCache
        result |= 0x200;
    }

    var ladder = new []
    {
        new { Flags = new [] { DataSectionFlags.MemoryExecute, DataSectionFlags.MemoryRead, DataSectionFlags.MemoryWrite, }, Value = (uint)0x40 },
        new { Flags = new [] { DataSectionFlags.MemoryExecute, DataSectionFlags.MemoryRead, }, Value = (uint)0x20 },
        new { Flags = new [] { DataSectionFlags.MemoryExecute, DataSectionFlags.MemoryWrite, }, Value = (uint)0x80 },
        new { Flags = new [] { DataSectionFlags.MemoryExecute, }, Value = (uint)0x10 },
        new { Flags = new [] { DataSectionFlags.MemoryRead, DataSectionFlags.MemoryWrite, }, Value = (uint)0x04 },
        new { Flags = new [] { DataSectionFlags.MemoryRead, }, Value = (uint)0x02 },
        new { Flags = new [] { DataSectionFlags.MemoryWrite, }, Value = (uint)0x08 },
        new { Flags = new DataSectionFlags[] { }, Value = (uint)0x01 },
    };

    result |= ladder.Where(x => x.Flags.All(y => characteristics.HasFlag(y))).First().Value;

    return result;
}
answered on Stack Overflow Nov 21, 2018 by Enigmativity • edited Nov 21, 2018 by Enigmativity

User contributions licensed under CC BY-SA 3.0