2
\$\begingroup\$

Is there a more graceful way to structure the code below?

The fingerPosition values are increments of 1.

The fingerMask values start at 1, then 2 and then increment in multiples of 2.

    private int getFingerMask(int fingerPosition)
    {
        int fingerMask = 0;

        if (fingerPosition == 0)
        {
            fingerMask = 1;
        }
        else if (fingerPosition == 1)
        {
            fingerMask = 2;
        }
        else if (fingerPosition == 2)
        {
            fingerMask = 4;
        }
        else if (fingerPosition == 3)
        {
            fingerMask = 8;
        }
        else if (fingerPosition == 4)
        {
            fingerMask = 16;
        }
        else if (fingerPosition == 5)
        {
            fingerMask = 32;
        }
        else if (fingerPosition == 6)
        {
            fingerMask = 64;
        }
        else if (fingerPosition == 7)
        {
            fingerMask = 128;
        }
        else if (fingerPosition == 8)
        {
            fingerMask = 256;
        }
        else if (fingerPosition == 9)
        {
            fingerMask = 512;
        }

        return fingerMask;
    }
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Welcome to Code Review! I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Feel free to give it a different title if there is something more appropriate. \$\endgroup\$ Jan 4 '18 at 16:42
2
\$\begingroup\$

Yes, this can be done simply by using a power of 2.

private int getFingerMask(int fingerPosition)
{
    if (fingerPosition >= 0 && fingerPosition < 10)
    {
        return (int)Math.Pow(2, fingerPosition);
    }
    else
    {
        return 0;
    }
}
\$\endgroup\$
1
\$\begingroup\$

In addition to @Aaron's answer, you can also use bit shifting. Also Naming Guidelines would expect the method to begin with a capital letter. This could also be reduced to a one-line method:

private int GetFingerMask(int fingerPosition) => (fingerPosition >= 0 && fingerPosition < 10) ? 1 << fingerPosition : 0;
\$\endgroup\$
1
\$\begingroup\$

These magic numbers are very magical. These feel like they should be enums:

// Perhaps there are better names for these positions? Thumb, Forefinger, etc.?
public enum FingerPosition
{
    Zero = 0,
    One,
    Two,
    Three,
    Four,
    Five,
    Six,
    Seven,
    Eight,
    Nine
}

// Again, better naming would be in order.
[Flags]
public enum FingerMask
{
    Unknown = 0,
    Zero = 1,
    One = 2,
    Two = 4,
    Three = 8,
    Four = 16,
    Five = 32,
    Six = 64,
    Seven = 128,
    Eight = 256,
    Nine = 512
}

Then something like a Dictionary to associate them as such:

private static readonly IDictionary<FingerPosition, FingerMask> _FingerMap = new Dictionary<FingerPosition, FingerMask>
{
    [FingerPosition.Zero] = FingerMask.Zero,
    [FingerPosition.One] = FingerMask.One,
    [FingerPosition.Two] = FingerMask.Two,
    [FingerPosition.Three] = FingerMask.Three,
    [FingerPosition.Four] = FingerMask.Four,
    [FingerPosition.Five] = FingerMask.Five,
    [FingerPosition.Six] = FingerMask.Six,
    [FingerPosition.Seven] = FingerMask.Seven,
    [FingerPosition.Eight] = FingerMask.Eight,
    [FingerPosition.Nine] = FingerMask.Nine,
};

Your code then becomes type-safe in this manner:

private FingerMask getFingerMask(FingerPosition fingerPosition)
{
    FingerMask fingerMask;

    if (_FingerMap.TryGetValue(fingerPosition, out fingerMask))
    {
        return fingerMask;
    }

    return FingerMask.Unknown;
}

You can then cast that result to int if you really need it somewhere else as that.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.