Refactoring Code That Makes You Go “Huh?”

I was in a Microsoft Teams meeting recently and this piece of code was dropped into the chat:

        public float Flags
        {
            get
            {
                if ((_Flags & (uint)Math.Pow(2, 0)) != 0)
                    return 1;
                else
                    return 0;
            }
        }

It made me go “Huh?” because it seemed ugly in a lot of ways, and I’m very much in favour of elegant, readable code. We’ve all done it, written something then looked at it the next day and thought “What was I thinking?” so I’m as guilty of producing this sort of code as anyone. The thing is, apart from being ugly it’s also limited in a way that could so easily be fixed.

Here’s the background, the flag in question is encoded along with others as single bits because it’s from software with a long history. Way back then memory was expensive and we had to be efficient with it.

Being a single bit flag and therefore either 0 or 1, true or false … why return a float? I’m guessing because this code was copied from the Internet or maybe even derived from something that Chat GPT regurgitated. Either way, the return type looks wrong.

That’s great but there’s a whole bunch of other flags in that uint that we might want to access. And surely the getter should return the value of _Flags, not the value of one bit?

Then there’s the problem of magic numbers, in this snippet:

(uint)Math.Pow(2, 0)

We’re always going to raise to a power of 2 so the first literal is correct but the magic number 0 means that we might be creating problems for later if that flag moves. Also we might want to test the value of different flags. Should we add functions for each of them with magic numbers?

Of course not, here’s my solution to the problem, there are of course many more and if you want to suggest them please do so in the comments:

    public enum BitFlags
    {
        EnableConveyor = 0,
        EnableGrinder = 1,
        PrintLabels = 2
        // etc, etc
    }

    public class AppFlags
    {
        private uint _Flags;

        public AppFlags(uint Flags)
        {
            _Flags = Flags;
        }

        public uint Flags
        {
            get => _Flags;
        }

        public bool ConveyorEnabled()
        {
            return (_Flags & (uint)Math.Pow(2, (int)BitFlags.EnableConveyor)) != 0;
        }
        public bool FlagIsSet(BitFlags bitFlag)
        {
            return (_Flags & (uint)Math.Pow(2, (int)bitFlag)) != 0;
        }
    }