3
\$\begingroup\$

This question is a follow up of Calculate min/max values for signed or unsigned integral number with any valid number of bits (up to 64)

I was answering the question Given two int values, return the one closer to 10, and I indicated that something like the following snippet of code could be a different approach for solving the problem.

In the original question, it turned out I had made a mistake, but since there were already some answers, I've decided to post a follow up question with the fixes.

Can this be done in a more efficient or more elegant way?

public class Bits
{
    public static void MinMaxI(ulong _bits, out long _min, out long _max)
    {
        // ? unsigned range with at least one value bit
        if (_bits < 2 || _bits > 64)
        {
            throw new System.ArgumentOutOfRangeException("_bits", _bits,
             String.Format("2 <= _bits <= 64"));
        }

        _min = ~(long)0 << (int)(_bits - 1);
        _max = -1 - _min;
    }


    public static long MinI(ulong _bits)
    {
        // ? unsigned range with at least one value bit
        if (_bits < 2 || _bits > 64)
        {
            throw new System.ArgumentOutOfRangeException("_bits", _bits,
             String.Format("2 <= _bits <= 64"));
        }

        return ~(long)0 << (int)(_bits - 1);
    }


    public static long MaxI(ulong _bits)
    {
        // ? unsigned range with at least one value bit
        if (_bits < 2 || _bits > 64)
        {
            throw new System.ArgumentOutOfRangeException("_bits", _bits,
             String.Format("2 <= _bits <= 64"));
        }

        return -1 - (~(long)0 << (int)(_bits - 1));
    }


    public static ulong MaxN(ulong _bits)
    {
        // ? unsigned range with at least one value bit
        if (_bits < 1 || _bits > 64)
        {
            throw new System.ArgumentOutOfRangeException("_bits", _bits,
             String.Format("1 <= _bits <= 64"));
        }

        return _bits == 64 ? ~(ulong)0 : ~(~(ulong)0 << (int)_bits);
    }
}
\$\endgroup\$
12
  • 1
    \$\begingroup\$ Would the question title "Calculate min/max values for (un)signed integral number of up to 64 bits" be a good title for your question, you think? \$\endgroup\$ Commented Apr 24, 2015 at 9:04
  • \$\begingroup\$ @Pimgd: that would be a better title. Can I change this without again violating any rules? Or do you have to do it? I didn't write this code in my answer to the other question, it's a different approach to the problem. So your edit is not correct. And the question "Can this be done in a more efficient or more elegant way?" is not allowed, according to Jamal, because it concerns code yet to be written. Perhaps you can give me an answer: what's the purpose of this site if only 100% correct and 100% efficient code may be posted? \$\endgroup\$ Commented Apr 24, 2015 at 9:12
  • \$\begingroup\$ You can edit your own question no problem, it's just that certain actions are prohibited. There's some obvious rules, like no offensive content, but you don't seem to be having trouble with those. What you're getting tripped up by are the rules we slowly defined as a community that serve to keep questions and answers valuable, even after several months or years have past. \$\endgroup\$ Commented Apr 24, 2015 at 9:17
  • 1
    \$\begingroup\$ You might be interested in reading this meta: meta.codereview.stackexchange.com/questions/5201/… - basically, there's a bit of a difference here. We're a pretty big site and receive a few code dumps or just plain broken code here on a daily basis and quickly filter these by closing the question. Sometimes it feels unfair. I think your case should be discussed though, it's not okay if the close message says you can fix the issue and then it turns out you can't. \$\endgroup\$ Commented Apr 24, 2015 at 9:48
  • 1
    \$\begingroup\$ @Pimgd: I didn't want to spend more than a day. But the question got me interested because I remembered that I ran into such edge case problems several times through the years. So I thought, it would be helpful for others to go deeper into the subject and I still think that my answer can be helpful to others in many ways. I like to chat with you because you are very reasonable - thanks for that. But we are violating the rules here, so let's cut it short. Perhaps another place another time - my pleasure. \$\endgroup\$ Commented Apr 24, 2015 at 10:09

1 Answer 1

Reset to default
3
\$\begingroup\$

Your argument validation could be de-duplicated.

Here, take a look:

MinMaxI:
    if (_bits < 2 || _bits > 64)
    {
        throw new System.ArgumentOutOfRangeException("_bits", _bits,
         String.Format("2 <= _bits <= 64"));
    }

MinI:
    if (_bits < 2 || _bits > 64)
    {
        throw new System.ArgumentOutOfRangeException("_bits", _bits,
         String.Format("2 <= _bits <= 64"));
    }

MaxI:
    if (_bits < 2 || _bits > 64)
    {
        throw new System.ArgumentOutOfRangeException("_bits", _bits,
         String.Format("2 <= _bits <= 64"));
    }

MaxN:
    if (_bits < 1 || _bits > 64)
    {
        throw new System.ArgumentOutOfRangeException("_bits", _bits,
         String.Format("1 <= _bits <= 64"));
    }

They're practically the same!

I'd make a method like this (I'm not really familiar with C# syntax, so let me know if I made a mistake)

private static void validateBitsArgument(ulong _bits, ulong min, ulong max)
{
    if (_bits < min || _bits > max)
    {
        throw new System.ArgumentOutOfRangeException("_bits", _bits,
         String.Format("{0} <= _bits <= {1}", min, max));
    }
}

And then include it like this:

public class Bits
{
    public static void MinMaxI(ulong _bits, out long _min, out long _max)
    {
        validateBitsArgument(_bits, 2, 64);

        _min = ~(long)0 << (int)(_bits - 1);
        _max = -1 - _min;
    }


    public static long MinI(ulong _bits)
    {
        validateBitsArgument(_bits, 2, 64);

        return ~(long)0 << (int)(_bits - 1);
    }


    public static long MaxI(ulong _bits)
    {
        validateBitsArgument(_bits, 2, 64);

        return -1 - (~(long)0 << (int)(_bits - 1));
    }


    public static ulong MaxN(ulong _bits)
    {
        validateBitsArgument(_bits, 1, 64);

        return _bits == 64 ? ~(ulong)0 : ~(~(ulong)0 << (int)_bits);
    }

    private static void validateBitsArgument(ulong _bits, ulong min, ulong max)
    {
        if (_bits < min || _bits > max)
        {
            throw new System.ArgumentOutOfRangeException("_bits", _bits,
         String.Format("{0} <= _bits <= {1}", min, max));
        }
    }
}

And voila, shorter code.

\$\endgroup\$
6
  • \$\begingroup\$ Answer written in 6-7 minutes - just a quick review of something that I spotted... and partially to demonstrate that it doesn't take a huge amount of time to help someone out if you can see something they don't. \$\endgroup\$ Commented Apr 24, 2015 at 10:08
  • \$\begingroup\$ And that's a really good one! The only thing to consider is, that an additional function call is more expensive than the code duplication. So perhaps an even better way would be a preprocessor macro? \$\endgroup\$ Commented Apr 24, 2015 at 10:34
  • 1
    \$\begingroup\$ @carpetemporem Perhaps, but if you go that way you should ditch the String.Format call too - just put the numbers straight in there. \$\endgroup\$ Commented Apr 24, 2015 at 10:36
  • 2
    \$\begingroup\$ @carpetemporem - and C# has a JIT compiler that can inline functions, so you might not need to do it manually anyways. \$\endgroup\$ Commented Apr 24, 2015 at 10:48
  • 2
    \$\begingroup\$ @Clockwork-Muse - and there's always the [MethodImpl(MethodImplOptions.AgressiveInlining)] attribute if the JIT doesn't figure it out. \$\endgroup\$ Commented Apr 24, 2015 at 10:53

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.