16
\$\begingroup\$

I did the exercise below to practice. Can someone help me to shorten or improve the code in a simple way for a beginner?

The exercise is:

Given 2 int values, return whichever value is nearest to the value 10, or return 0 in the event of a tie.

public int Closer(int a, int b) 
{  
     Console.WriteLine(Closer(8, 13));    // output: 8
     Console.WriteLine(Closer(13, 8));    // output: 8 
     Console.WriteLine(Closer(13, 7));    // output: 0
}

My solution:

namespace Closer
    {
        class Program
        {
            static void Main(string[] args)
            {
                Console.WriteLine(Closer(13,15));    // output : 13
                Console.WriteLine(Closer(13, 13));   // output : 0
                Console.WriteLine(Closer(3,5));      // output : 5
                Console.WriteLine(Closer(13, 7));    // output : 0
                Console.WriteLine(Closer(-3, -5));   // output : -3
                Console.WriteLine(Closer(-3, -1));   // output : -1
                Console.ReadLine();
            }
            public static int Closer(int a, int b)
            {
                int calcA = a - 10;
                int calcB = b - 10;
                if (Math.Abs(calcA) == Math.Abs(calcB))
                {
                    return 0;
                }
                else if ((a >= 10 || b >= 10) && a < b )
                {
                    return a;
                }
                else if ((a >= 10 || b >= 10) && b < a)
                {
                    return b;
                }
                else if (calcB > calcA && Math.Abs(a)!= Math.Abs(b))
                {
                    return b;
                }
                else if ((calcA < 0 || calcB < 0) && (calcA > calcB && Math.Abs(a) != Math.Abs(b)))
                {
                    return a;
                }
                else return Closer(a,b);
            }
        }
    }
\$\endgroup\$
6
  • 6
    \$\begingroup\$ \$\mid x -y\mid\$ represents the distance between \$x\$ and \$y\$. You should not need to perform multiple branches. \$\endgroup\$ Apr 13, 2015 at 15:58
  • 16
    \$\begingroup\$ I would note that the statement of the problem requires you to produce an extremely poorly designed method. Suppose you have Compare(0, 21). This should produce zero because zero is closer, not because the distances are the same. Basically the value "zero" is being overloaded to mean both "this is the closer value" and "there is no closer value". A good follow-up exercise would be to design a better method that does not have this problem. \$\endgroup\$ Apr 13, 2015 at 18:15
  • 2
    \$\begingroup\$ Another good exercise for you: C# now has a BigInteger type in System.Numerics which can represent arbitrarily large integers. How would you write your method if it took BigIntegers instead of ints? Bonus: how can you write the code so that it is obviously a solution to the stated problem? The biggest problem with your original code, aside from its incorrectness, is that even if it were correct, it is impossible to glance at it and understand that it is correct immediately. \$\endgroup\$ Apr 13, 2015 at 19:06
  • \$\begingroup\$ @EricLippert: You should mention that to the guys who write PHP. \$\endgroup\$
    – dotancohen
    Apr 13, 2015 at 19:49
  • 2
    \$\begingroup\$ @dotancohen: Indeed, that library method is a mess. PHP as a whole is more than a little dodgy from a language design viewpoint; as its creators freely admit, they were not professional language designers and the whole thing grew organically in an ad-hoc fashion. \$\endgroup\$ Apr 13, 2015 at 19:55

7 Answers 7

30
\$\begingroup\$

You have a lot of redundant code in here:

public static int Closer(int a, int b)
{
    int calcA = a - 10;
    int calcB = b - 10;
    if (Math.Abs(calcA) == Math.Abs(calcB))
    {
        return 0;
    }
    else if ((a >= 10 || b >= 10) && a < b )
    {
        return a;
    }
    else if ((a >= 10 || b >= 10) && b < a)
    {
        return b;
    }
    else if (calcB > calcA && Math.Abs(a)!= Math.Abs(b))
    {
        return b;
    }
    else if ((calcA < 0 || calcB < 0) && (calcA > calcB && Math.Abs(a) != Math.Abs(b)))
    {
        return a;
    }
    else return Closer(a,b);
}

First of all, you can figure everything out if you know the distance between the values of a, b, and 10.

int calcA = Math.Abs(a - 10);
int calcB = Math.Abs(b - 10);

You have principle in this section right, but it can be simplified a bit if you set calcA and calcB to use the absolute difference:

if (calcA == calcB)
{
    return 0;
}

Now, you just need to apply the same principle to the other values:

if (calcA < calcB)
{
    return a;
}

return b;    // we already established that the values are not the same, and that "a" is farther from 10 than "b"

Here is our new method:

public static int Closer(int a, int b)
{
    int calcA = Math.Abs(a - 10);
    int calcB = Math.Abs(b - 10);

    if (calcA == calcB)
    {
        return 0;
    }

    if (calcA < calcB)
    {
        return a;
    }

    return b;
}

But, 10 is a magic number in there, and magic numbers are never good. Why don't you create a const value to store the number that you are checking the passed values against?

public static int Closer(int a, int b)
{
    const int compareValue = 10;
    int calcA = Math.Abs(a - compareValue);
    int calcB = Math.Abs(b - compareValue);

    if (calcA == calcB)
    {
        return 0;
    }

    if (calcA < calcB)
    {
        return a;
    }

    return b;
}

Now, you can change your method as much as you want, and you only have the magic number 10 in one place, so it is easy to change your method to compare the values against a different number.

Edit:

Thanks to Eric Lippert, I have fixed two bugs. Eric found the bug that int.MinValue != int.MaxValue, so calling Math.Abs(int.MinValue) crashes. If you pass a value that equals int.MinValue + compareValue, the program will crash. While fixing this, I found another bug in which the wrong value will be returned if you call the function with two values that are less than int.MinValue + compareValue because they wrap around in the call value - compareValue. Both of these bugs are now fixed for the compare value of 10, still working on a generic patch:

public static int Closer(int a, int b)
{
    const int compareValue = 10;

    while (a <= int.MinValue + compareValue || b <= int.MinValue + compareValue)
    {
        if (a <= int.MinValue + compareValue)
        {
            b = AdjustVal(b, compareValue);
            a++;
        }
        if (b <= int.MinValue + compareValue)
        {
            a = AdjustVal(a, compareValue);
            b++;
        }
    }

    int calcA = Math.Abs(a - compareValue);
    int calcB = Math.Abs(b - compareValue);

    if (calcA == calcB)
    {
        return 0;
    }

    if (calcA < calcB)
    {
        return a;
    }

    return b;
}

Edit 2:

Eric Lippert says we should do the arithmetic in longs. Of course, this doesn't work when we take input as long too.

public static int Closer(int a, int b)
{
    const int compareValue = 10;

    long calcA = Math.Abs((long)a - compareValue);
    long calcB = Math.Abs((long)b - compareValue);

    if (calcA == calcB)
    {
        return 0;
    }

    if (calcA < calcB)
    {
        return a;
    }

    return b;
}
\$\endgroup\$
2
  • 3
    \$\begingroup\$ There was an easier way to fix your bug: do all the arithmetic in longs. \$\endgroup\$ Apr 13, 2015 at 18:59
  • 1
    \$\begingroup\$ Minor style issue: I suggest that you use else construct in the last if statement to preserve the a/b symmetry (makes it clearer to the reader what we are doing). \$\endgroup\$
    – orion
    Apr 15, 2015 at 12:13
23
\$\begingroup\$

Your code crashes when given valid inputs, and is therefore automatically wrong. The test case

Closer(int.MinValue + 10, 1)

clearly should produce a result of one, but it crashes the program.

The other answers all have similar crashing cases, and so they are all wrong too. (UPDATE: Fixes are in progress, and a seemingly correct solution has been posted as well.)

Writing code that solves this problem that actually works across the entire range of integers is a bit tricky. See if you can do so!

Remember, make the code correct first, and then make it elegant, and only then consider whether it is efficient. Neither you nor anyone else who has answered so far has gotten past "correct", so don't worry about its inelegance yet.

\$\endgroup\$
6
  • \$\begingroup\$ @Lyle'sMug: I don't understand your question. int.MinValue + 10 is equal to -2147483638. My point is that the given test case crashes the original poster's program. A different test case crashes your program; can you find it? \$\endgroup\$ Apr 13, 2015 at 18:11
  • \$\begingroup\$ I saw it, and deleted my comment, I haven't finished setting up my tests for my code yet, so no I haven't found it, yet. \$\endgroup\$
    – Malachi
    Apr 13, 2015 at 18:12
  • \$\begingroup\$ @Lyle'sMug: int.MinValue is an illegal input to Abs. Your input to Abs is 10-a. What value must a have for 10-a to equal MinValue ? \$\endgroup\$ Apr 13, 2015 at 18:19
  • \$\begingroup\$ @Nova "Has a bug" is only somewhat useful in that it quantifies how many errors the code has (but fails to account for its gravity). No need to mince words. "The animistic metaphor of the bug that maliciously sneaked in while the programmer was not looking is intellectually dishonest as it disguises that the error is the programmer's own creation. The nice thing of this simple change of vocabulary is that it has such a profound effect: while, before, a program with only one bug used to be "almost correct", afterwards a program with an error is just "wrong" (because in error)." -EWD 1036 \$\endgroup\$
    – Doval
    Apr 14, 2015 at 13:29
  • 3
    \$\begingroup\$ @Nova: a thing cannot be both almost correct and correct. If it is almost correct then it is wrong. \$\endgroup\$ Apr 14, 2015 at 15:59
7
\$\begingroup\$

Following @Eric Lippert's suggestion, and passing the compare value of 10 as an input parameter, I have this example:

    public static int Closer(int a, int b, int compareValue = 10)
    {
        long along = a;
        long blong = b;
        long clong = compareValue;

        var distanceA = Math.Abs(clong - along);
        var distanceB = Math.Abs(clong - blong);

        if (distanceA == distanceB)
        {
            return 0;
        }

        return distanceA > distanceB ? b : a;
    }
\$\endgroup\$
3
  • \$\begingroup\$ I haven't tested it but this solution appears to be correct. \$\endgroup\$ Apr 13, 2015 at 19:49
  • \$\begingroup\$ Oh I like this trick a lot. I was thinking in the area of unchecked arithmetic. \$\endgroup\$ Apr 13, 2015 at 20:00
  • \$\begingroup\$ @JeroenVannevel Unchecked arithmetic opens up a whole new can of worms. It would be nice if we could finally throw away notions like "it matters that the numeric value fits 32 bits and negative numbers are represented like so" :D \$\endgroup\$
    – Luaan
    Apr 15, 2015 at 7:40
7
\$\begingroup\$

I kind of went straight for the distance from 10 approach like @Hosch did, except for I like shorter code that says the same thing, and I know this could be shorter but here we go.

you had a lot of else if statements here, really all you wanted to know is which number is closer to 10, so of course we find out first if the numbers are the same, because if they are then there is no reason whatsoever to perform math or assign variables, we just dump out right there.

Then we calculate the the absolute value of the distance from 10, I did use a magic number here, if you wanted to make it more extendable then you could add another parameter to the Method and use that in place of 10 throughout the method. I subtracted each given number from 10 and then removed the sign of the integer. then it is a simple, which distance is greater (or, lesser or equal)

    public static int Closer_LylesMug(int a, int b)
    {
        long distanceA = Math.Abs(10 - (long)a);
        long distanceB = Math.Abs(10 - (long)b);

        if (distanceA == distanceB)
        {
            return 0;
        }

        return distanceA > distanceB ? b : a;
    }

Edited code: yes I caved and removed the early dumpout. I also fixed the little int.Min issue as well, Thank you Eric Lippert for pointing out the Edge cases to us.

I am going to boast and say that this code is probably still the cleanest code except for that it really is a tie with @Dain II Ironfoot's code.

  • Rick's code creates 2 extra unneeded variables
  • Dain II Ironfoot's code is almost exactly the same if you spread out the Ternary statement of mine into an if then statement.
\$\endgroup\$
3
  • 1
    \$\begingroup\$ I'd agree with removing the first check, there's no reason to think the microseconds is shaves are relevant here. I do prefer the ternary and the somewhat better variable naming \$\endgroup\$ Apr 13, 2015 at 16:48
  • 1
    \$\begingroup\$ return (distanceA == distanceB) ? 0 : ((distanceA > distanceB) ? b : a); \$\endgroup\$
    – WernerCD
    Apr 14, 2015 at 1:25
  • 1
    \$\begingroup\$ I could do it like that, but I think it makes the code a little harder to read. \$\endgroup\$
    – Malachi
    Apr 14, 2015 at 1:27
5
\$\begingroup\$

A little math can shorten this. Take the average of the two numbers. If the average is higher than 10, that means the higher number is pulling up the average more than the lower number is pulling it down. So return the lower number. If lower, then return higher. If 10, return 0.

public static int Closer(int a, int b)
{
    double avg = ((double)a + (double)b) / 2.0;
    return (a == b || avg == 10.0) ? 0 :
           avg > 10.0 ? Math.Min(a,b) : Math.Max(a,b);
}

This may not be the best solution because it's not immediately clear what this code is doing.

Edit: Made a small change in the case that a == b is true. I now treat that the same as how I treat two numbers equidistant from 10: I return 0. Thanks Lyle's Mug

\$\endgroup\$
3
  • 1
    \$\begingroup\$ I think this is wrong. -10 + -10 = -20 and -20 / 2 = -10 and -10 < 10 and -10 <>10 \$\endgroup\$
    – Malachi
    Apr 13, 2015 at 20:48
  • 1
    \$\begingroup\$ Use long sum=a+b and compare with 20 to avoid floating point arithmetic. \$\endgroup\$
    – Taemyr
    Apr 14, 2015 at 7:53
  • \$\begingroup\$ You might be able to do the calculation in integer arithmetic; stackoverflow.com/a/5700904 has this formula: a/2 + b/2 + (a%2 + b%2)/2. \$\endgroup\$
    – Neil
    Apr 14, 2015 at 23:08
3
\$\begingroup\$

I think it's not correct to switch to long to go around int limits.

What if you want to do the same with long inputs?

You can do that by using double internally or by transforming the delta-calculating formulas. Forget about using double internally since it can't represent the range of long and ulong. So Closer.CheckPrint and Closer.Check won't work with huge long and ulong values. Thereby Closer.I and Closer.N are working solutions. Closer.CheckI and Closer.CheckN could be written using using BitsMinMaxI and BitsMinMaxN respectively all using long and ulong inputs and internal variables respectively. BitsMinMaxI and BitsMinMaxN have to be written with bit shift (<<) instead of Math.Pow. << is tricky again since max shift in C# is 63!

A solution for any integral values (with any valid number of bits for signed and unsigned values) is shown in Closer.CheckPrint with optional checking of the values and calculations and optional printing of the formula transformation steps.

A 'real-world' function, always checking the range of the input values and without printing is Closer.Check.

Both of them internally work with doubles. You could do Closer.CheckPrint without all the different cases and transformations. But it's done like this as a preparation for the following integral internal calculations.

More efficency is gained by doing integral calculations internally. Due to the range of the input values, you need two functions, one for signed values (Closer.I - I standing for integer), working internally with long and one for unsigned values (Closer.N - N standing for natural), working internally with ulong.

Closer.N is simpler because no overflow is possible in the delta-calculations.

For Closer.I as written below, it should be noted that the order of evaluation (which is from left to right for + and - in C#) and the order of operations in the two cases where _one is negative and _two is positive are critical. E.g. if you use delta_one_ = _value - _one - _two; instead of delta_one_ = _value - _two - _one; the intermediate result _value - _one would overflow if _value == 0 and _one == min. If you do the same in languages for which the evaluation order is not guranteed (e.g. C or C++), you have to do the calculation in two steps (e.g. delta_one_ = _value - _two; delta_one_ -= _one;).

The compiler can do the input type checking if you want to check for less than 8 byte inputs. This is shown by Closer.[IN][1248] for signed (I) and unsigned (N) inputs with 1, 2, 4, or 8 bytes. All of them call Closer.[IN], just converting the return value for less than 8 bytes.

Closer.I8 just calls Closer.I and Closer.N8 just calles Closer.N without any further ado. So you could also drop Closer.I and Closer.N and insert the code in Closer.I8 and Closer.N8 respectively. But I think it's clearer to have a function for every possible byte-count and one functions working for all byte-counts.

I don't have a local compiler at hand but from online checking the code should work.

using System;
using System.Linq;


class Closer
{
    // integral signed (_bits < -1) or unsigned (_bits > 0) values
     // with specified number of bits (including sign bit for signed values)
     // optionally check input range and print steps

    public static double CheckPrint(
     double _one, double _two, double _value, long _bits, out bool _is_closer,
     bool _check = true, bool _print = true)
    {
        double min_ = 0, max_ = 0;
        Helper.BitsMinMax(_bits, out min_, out max_);

        int indent_ = 2;

        // check range of inputs:

        Helper.PrintRange(_one, min_, max_, _print, 1, "_one", 1);
        Helper.CheckIntegerRange(_one, min_, max_, _check);
        Helper.PrintRange(_two, min_, max_, _print, 0, "_two", 1);
        Helper.CheckIntegerRange(_two, min_, max_, _check);
        Helper.PrintRange(_value, min_, max_, _print, 0, "_value", 2);
        Helper.CheckIntegerRange(_value, min_, max_, _check);

        // ? shortcut
        if (_one == _two)
        {
            Helper.Print(_print, 0, "_one == _two", 2);

            _is_closer = false;
            return 0;
        }

        // ! _one != _two

        // ? swap values to ensure _one < _two
        if (_two < _one)
        {
            Helper.Print(_print, 0, "swap _one _two", 2);

            double tmp_ = _one;
            _one = _two;
            _two = tmp_;
        }

        // ? shortcut: _value <= _one < _two
        if (_value <= _one)
        {
            Helper.Print(_print, 0, "_value <= _one < _two", 2);

            _is_closer = true;
            return _one;
        }

        // ? shortcut: _one < _two <= _value
        if (_value >= _two)
        {
            Helper.Print(_print, 0, "_one < _value <= _two", 2);

            _is_closer = true;
            return _two;
        }

        // ! _one != _two and _one <= _value <= _two

        double delta_one_;
        double delta_two_;

        // ? 0 <= _one <= _value <= _two or _one <= _value <= two < 0:
        if (_one >= 0 || _two < 0)
        {
            // ! same sign: no overflow possible:

            Helper.Print(_print, 0,
             "0 <= _one <= _value <= _two or _one <= _value <= two < 0", 2);

            // edge case _one == min, _value == -1: max
            Helper.PrintRange(_value - _one, min_, max_, _print,
             0, "delta_one_", 1, 1 * indent_);
            Helper.CheckRange(_value - _one, min_, max_, _check);
            // edge case _two == max, _value == 0: max
            Helper.PrintRange(_two - _value, min_, max_, _print,
             0, "delta_two_", 2, 1 * indent_);
            Helper.CheckRange(_two - _value, min_, max_, _check);

            delta_one_ = _value - _one;
            delta_two_ = _two - _value;
        }
        // ? _one < 0 <= _two
        else
        {
            Helper.Print(_print, 0, "_one < 0 <= _two", 2);

            // ! different sign: overflow possible!

            // ? delta_two_ is save
            if (_value >= 0)
            {
                Helper.Print(_print, 0, "_value >= 0", 2, 1 * indent_);

                // fail: _one == min, _value >= 0
                double test_delta_one_ = _value - _one;
                Helper.PrintRange(test_delta_one_, min_, max_,
                 _print, 0, "0 delta_one_", 1, 2 * indent_);

                // save: _two == max, _value == 0 (edge case)
                double test_delta_two_ = _two - _value;
                Helper.PrintRange(test_delta_two_, min_, max_,
                 _print, 0, "0 delta_two_", 2, 2 * indent_);
                Helper.CheckRange(test_delta_two_, min_, max_, _check);

                // decrease both deltas by save delta_two_
                 // (so delta_two_ becomes obviously 0):

                test_delta_one_ = test_delta_one_ - test_delta_two_;
                Helper.PrintRange(test_delta_one_, min_, max_,
                 _print, 0, "1 delta_one_", 1, 3 * indent_);

                test_delta_one_ = (_value - _one) - (_two - _value);
                Helper.PrintRange(test_delta_one_, min_, max_,
                 _print, 0, "2 delta_one_", 1, 3 * indent_);

                test_delta_one_ = _value - _one - _two + _value;
                Helper.PrintRange(test_delta_one_, min_, max_,
                 _print, 0, "3 delta_one_", 1, 3 * indent_);

                test_delta_one_ = 2 * _value - _one - _two;
                Helper.PrintRange(test_delta_one_, min_, max_,
                 _print, 0, "4 delta_one_", 1, 3 * indent_);

                test_delta_two_ = 0;
                Helper.PrintRange(test_delta_two_, min_, max_,
                 _print, 0, "1 delta_two_", 2, 3 * indent_);

                // decrease both deltas by _value

                test_delta_one_ = 2 * _value - _one - _two - _value;
                Helper.PrintRange(test_delta_one_, min_, max_,
                 _print, 0, "5 delta_one_", 1, 4 * indent_);
                Helper.CheckRange(test_delta_one_, min_, max_, _check);

                test_delta_one_ = _value - _one - _two;
                Helper.PrintRange(test_delta_one_, min_, max_,
                 _print, 0, "6 delta_one_", 1, 4 * indent_);
                Helper.CheckRange(test_delta_one_, min_, max_, _check);

                test_delta_two_ = -_value;
                Helper.PrintRange(test_delta_two_, min_, max_,
                 _print, 0, "2 delta_two_", 2, 4 * indent_);
                Helper.CheckRange(test_delta_two_, min_, max_, _check);

                delta_one_ = _value - _two - _one;
                delta_two_ = -_value;
            }
            // ? delta_one_ is save
            else
            {
                Helper.Print(_print, 0, "_value < 0", 2, 1 * indent_);

                // save: _one == min, _value == -1 (edge case)
                double test_delta_one_ = _value - _one;
                Helper.PrintRange(test_delta_one_, min_, max_,
                 _print, 0, "0 delta_one_", 1, 2 * indent_);
                Helper.CheckRange(test_delta_one_, min_, max_, _check);

                // fail: _two == max, _value < 0
                double test_delta_two_ = _two - _value;
                Helper.PrintRange(test_delta_two_, min_, max_,
                 _print, 0, "0 delta_two_", 2, 2 * indent_);

                // decrease both deltas by save delta_one_
                 // (so delta_one_ becomes obviously 0):

                test_delta_one_ = 0;
                Helper.PrintRange(test_delta_one_, min_, max_,
                 _print, 0, "1 delta_one_", 1, 3 * indent_);
                Helper.CheckRange(test_delta_one_, min_, max_, _check);

                test_delta_two_ = test_delta_two_ - test_delta_one_;
                Helper.PrintRange(test_delta_two_, min_, max_,
                 _print, 0, "1 delta_two_", 1, 3 * indent_);
                test_delta_two_ = (_two - _value) - (_value - _one);
                Helper.PrintRange(test_delta_two_, min_, max_,
                 _print, 0, "2 delta_two_", 1, 3 * indent_);
                test_delta_two_ = _two - _value - _value + _one;
                Helper.PrintRange(test_delta_two_, min_, max_,
                 _print, 0, "3 delta_two_", 1, 3 * indent_);
                test_delta_two_ = _two + _one - 2 * _value;
                Helper.PrintRange(test_delta_two_, min_, max_,
                 _print, 0, "4 delta_two_", 2, 3 * indent_);

                // increase both deltas by _value:

                test_delta_one_ = _value;
                Helper.PrintRange(test_delta_one_, min_, max_,
                 _print, 0, "2 delta_one_", 1, 4 * indent_);
                Helper.CheckRange(test_delta_one_, min_, max_, _check);

                test_delta_two_ = _two + _one - 2 * _value + _value;
                Helper.PrintRange(test_delta_two_, min_, max_,
                 _print, 0, "5 delta_two_", 1, 4 * indent_);
                Helper.CheckRange(test_delta_two_, min_, max_, _check);
                test_delta_two_ = _two + _one - _value;
                Helper.PrintRange(test_delta_two_, min_, max_,
                 _print, 0, "6 delta_two_", 2, 4 * indent_);
                Helper.CheckRange(test_delta_two_, min_, max_, _check);

                delta_one_ = _value;
                delta_two_ = _one - _value + _two;
            }
        }

        if (delta_one_ < delta_two_)
        {
            _is_closer = true;
            return _one;
        }

        if (delta_two_ < delta_one_)
        {
            _is_closer = true;
            return _two;
        }

        _is_closer = false;
        return 0;
    }


    // integral signed (_bits < -1) or unsigned (_bits > 0) values
     // with specified number of bits (including sign bit for signed values):

    public static double Check(
     double _one, double _two, double _value, long _bits, out bool _is_closer)
    {
        double min_ = 0, max_ = 0;
        Helper.BitsMinMax(_bits, out min_, out max_);

        // check range of inputs:

        Helper.CheckIntegerRange(_one, min_, max_, true);
        Helper.CheckIntegerRange(_two, min_, max_, true);
        Helper.CheckIntegerRange(_value, min_, max_, true);

        if (_two < _one)
        {
            double tmp_ = _one;
            _one = _two;
            _two = tmp_;
        }

        double delta_one_ = _value - _one;
        double delta_two_ = _two - _value;

        if (delta_one_ < delta_two_)
        {
            _is_closer = true;
            return _one;
        }

        if (delta_two_ < delta_one_)
        {
            _is_closer = true;
            return _two;
        }

        _is_closer = false;
        return 0;
    }


    // all signed ('I' for integer) integral types:

    public static long I(
     long _one, long _two, long _value, out bool _is_closer)
    {
        // ? shortcut
        if (_one == _two)
        {
            _is_closer = false;
            return 0;
        }

        // ! _one != _two

        // ? swap values to ensure _one < _two
        if (_two < _one)
        {
            _one ^= _two;
            _two ^= _one;
            _one ^= _two;
        }

        // ? shortcut: _value <= _one < _two
        if (_value <= _one)
        {
            _is_closer = true;
            return _one;
        }

        // ? shortcut: _one < _two <= _value
        if (_value >= _two)
        {
            _is_closer = true;
            return _two;
        }

        // ! _one != _two and _one < _value < _two

        long delta_one_;
        long delta_two_;

        // ? 0 <= _one <= _value <= _two or _one <= _value <= two < 0:
        if (_one >= 0 || _two < 0)
        {
            // same sign: no overflow possible

            delta_one_ = _value - _one;
            delta_two_ = _two - _value;
        }
        // ? _one < 0 <= _two
        else
        {
            // different sign: overflow possible!

            // ? delta_two_ is save
            if (_value >= 0)
            {
                // to both deltas: - delta_two_ - _value

                delta_two_ = -_value;

                // min = -(max + 1) and min <= _one <= -1 and 0 <= _two <= max
                 // and 0 <= _value < _two and evaluation from left to right:

                 // -max <= _value - _two <= -1 and -1 - max <= _one <= -1
                 // -max + 1 <= (-max .. -1) - (-1 - max .. -1) <= max
                delta_one_ = _value - _two - _one;
            }
            // ? delta_one_ is save
            else
            {
                // to both deltas: - delta_one_ + _value

                delta_one_ = _value;

                // min = -(max + 1) and min <= _one <= -1 and 0 <= _two <= max
                 // _one < _value <= -1 and evaluation from left to right:

                 // -max <= _one - _value <= -1 and 0 <= _two <= max
                 // -max <= (-max .. -1) + (0 .. max) <= max - 1
                delta_two_ = _one - _value + _two;
            }
        }

        if (delta_one_ < delta_two_)
        {
            _is_closer = true;
            return _one;
        }

        if (delta_two_ < delta_one_)
        {
            _is_closer = true;
            return _two;
        }

        _is_closer = false;
        return 0;
    }


    // all unsigned ('N' for natural) integral types:

    public static ulong N(
     ulong _one, ulong _two, ulong _value, out bool _is_closer)
    {
        // ? shortcut
        if (_one == _two)
        {
            _is_closer = false;
            return 0;
        }

        // ! _one != _two

        // ? swap values to ensure _one < _two
        if (_two < _one)
        {
            _one ^= _two;
            _two ^= _one;
            _one ^= _two;
        }

        // ? shortcut: _value <= _one < _two
        if (_value <= _one)
        {
            _is_closer = true;
            return _one;
        }

        // ? shortcut: _one < _two <= _value
        if (_value >= _two)
        {
            _is_closer = true;
            return _two;
        }

        // ! _one != _two and _one <= _value <= _two

        ulong delta_one_ = _value - _one;
        ulong delta_two_ = _two - _value;

        if (delta_one_ < delta_two_)
        {
            _is_closer = true;
            return _one;
        }

        if (delta_two_ < delta_one_)
        {
            _is_closer = true;
            return _two;
        }

        _is_closer = false;
        return 0;
    }


    public static sbyte I1(
     sbyte _one, sbyte _two, sbyte _value, out bool _is_closer)
    {
        return (sbyte)I(_one, _two, _value, out _is_closer);
    }


    public static short I2(
     short _one, short _two, short _value, out bool _is_closer)
    {
        return (short)I(_one, _two, _value, out _is_closer);
    }


    public static int I4(
     int _one, int _two, int _value, out bool _is_closer)
    {
        return (int)I(_one, _two, _value, out _is_closer);
    }


    public static long I8(
     long _one, long _two, long _value, out bool _is_closer)
    {
        return I(_one, _two, _value, out _is_closer);
    }


    public static byte N1(
     byte _one, byte _two, byte _value, out bool _is_closer)
    {
        return (byte)N(_one, _two, _value, out _is_closer);
    }


    public static ushort N2(
     ushort _one, ushort _two, ushort _value, out bool _is_closer)
    {
        return (ushort)N(_one, _two, _value, out _is_closer);
    }


    public static uint N4(
     uint _one, uint _two, uint _value, out bool _is_closer)
    {
        return (uint)N(_one, _two, _value, out _is_closer);
    }


    public static ulong N8(
     ulong _one, ulong _two, ulong _value, out bool _is_closer)
    {
        return N(_one, _two, _value, out _is_closer);
    }
}

Here are the Helper class and some sample calls in Test.Main.

public class Helper
{
    public static void BitsMinMax(long _bits, out double _min, out double _max)
    {
        // ? unsigned range with at least one value bit
        if (_bits > 0)
        {
            _max = Math.Pow(2, _bits) - 1;
            _min = 0;

            return;
        }

        // ? signed range with one sign bit and at least one value bit
        if (_bits < -1)
        {
            _min = -Math.Pow(2, -_bits - 1);
            _max = -1 - _min;

            return;
        }

        throw new System.ArgumentOutOfRangeException("_bits", _bits,
         String.Format("-64 <= _bits <= -2 or 1 <= _bits <= 64"));
    }


    public static void CheckRange(
     double _value, double _min, double _max, bool _check = true)
    {
        if (_check && (_value < _min || _value > _max))
        {
            throw new System.ArgumentOutOfRangeException(
             "_value", _value, ValueRange2String(_value, _min, _max));
        }
    }


    public static void CheckIntegerRange(
     double _value, double _min, double _max, bool _check = true)
    {
        CheckRange(_value, _min, _max, _check);

        if (!(_value < 0 && Math.Ceiling(_value) == _value
         || _value >= 0 && Math.Floor(_value) == _value))
        {
            throw new System.ArgumentException(
             String.Format("_value ({0}) must be integral", _value));
        }
    }


    public static string ValueRange2String(
     double _value, double _min, double _max)
    {
        string format_;

        if (_value < _min)
        {
            format_ = "_value ({0}) < _min ({1}) <= _max ({2})";
        }
        else if (_value > _max)
        {
            format_ = "_min ({1}) <= _max ({2}) < _value ({0})";
        }
        else
        {
            format_ = "_min ({1}) <= _value ({0}) <= _max ({2})";
        }

        return String.Format(format_, _value, _min, _max);
    }


    public static void Print(bool _print = true,
     int _head_lines = 0, string _text = "", int _tail_lines = 0,
     int _indent_count = 0, string _indent_string = " ")
    {
        if (_print)
        {
            Console.Write(String.Format("{0}{1}{2}{3}",
             new String ('\n', _head_lines),
             string.Concat(Enumerable.Repeat(_indent_string, _indent_count)),
             _text, new String ('\n', _tail_lines)));
        }
    }


    public static void PrintRange(
     double _value, double _min, double _max, bool _print = true,
     int _head_lines = 0, string _text = "", int _tail_lines = 0,
     int _indent_count = 0, string _indent_string = " ")
    {
        if (_print)
        {
            if (_text != "")
            {
                _text = String.Format("{0}: ", _text);
            }

            Print(true, _head_lines, String.Format("{0}{1}",
             _text, ValueRange2String(_value, _min, _max)), _tail_lines,
             _indent_count, _indent_string);
        }
    }


    public static void PrintCheckRange(
     double _value, double _min, double _max, bool _print = true,
     int _head_lines = 0, string _text = "", int _tail_lines = 0,
     int _indent_count = 0, string _indent_string = " ", bool _check = true)
    {
        PrintRange(_value, _min, _max, _print,
         _head_lines, _text, _tail_lines, _indent_count, _indent_string);
        CheckRange(_value, _min, _max, _check);
    }
}


class Test
{
    public static void Main()
    {
        bool is_closer_;

        Helper.Print(true, 1, String.Format("{0} {1}",
         Closer.CheckPrint(-8, 7, 6, -4, out is_closer_), is_closer_), 1);

        Helper.Print(true, 1, String.Format("{0} {1}",
         Closer.Check(-8, 7, 6, -4, out is_closer_), is_closer_), 1);

        Helper.Print(true, 1, String.Format("{0} {1}",
         Closer.CheckPrint(15, 0, 14, 4, out is_closer_), is_closer_), 1);

        Helper.Print(true, 1, String.Format("{0} {1}",
         Closer.Check(15, 0, 14, 4, out is_closer_), is_closer_), 1);

        Helper.Print(true, 1, String.Format("{0} {1}",
         Closer.I1(-128, 127, 126, out is_closer_), is_closer_), 1);

        Helper.Print(true, 1, String.Format("{0} {1}",
         Closer.N2(0, 254, 255, out is_closer_), is_closer_), 1);
    }
}
\$\endgroup\$
4
  • \$\begingroup\$ Sorry folks, my follow-up post with the Helper class and the sample calls was deleted by moderator Jamal. Beside that the Print... functions are not exactly problem related, these and at least the BitsMinMax function and the CheckIntegerRange function would have been helpful for understanding the problem at hand and for dealing with similar problems. Just wanted to help with further insight but that seems not to be welcome. \$\endgroup\$ Apr 22, 2015 at 15:21
  • 3
    \$\begingroup\$ That post was deleted because it was not an answer - you could put the other code in a PasteBin and post the link in your answer. \$\endgroup\$
    – user34073
    Apr 22, 2015 at 17:07
  • 1
    \$\begingroup\$ You are right and you are not. It was not "the" answer, it was part of the answer. Whts th wrth f n nswr f prts r lft t? Some parts left out ;) But thanks for the advice anyway. I'll go around that and create a PasteBin. \$\endgroup\$ Apr 22, 2015 at 21:30
  • \$\begingroup\$ @200_success: thank for integrating the Helper class. I was not able to do that because of size limitations. But to kick out my Edit note, makes the post illogical. Anyway that's my last comment or post here. Have fun, a happy live and thanks for the fish ;) \$\endgroup\$ Apr 24, 2015 at 18:20
-2
\$\begingroup\$

Simpler for the machine:

public int CloserTo(int a, int b, ref bool tie_flag, int referencePoint = 10)
{
    // Quick away trivial solution
    if (a == b)
    {
        tie_flag = true;
        return 0;
    }
    // Init tie flag
    tie_flag = false;
    // Case 1: {a...b...referencePoint} or {b...a...referencePoint}
    if ((a <= referencePoint) && (b <= referencePoint))
        return (a < b)? b : a;
    // Case 2: {referencePoint...a...b} or {referencePoint...b...a}
    if ((a >= referencePoint) && (b >= referencePoint))
        return (a < b)? a : b;

    long diff1, diff2;
    // Case 3: {a...referencePoint...b}
    if (a < b)
    {
        diff1 = (long) referencePoint-a;
        diff2 = (long) b-referencePoint;
        if (diff1 == diff2)
        {
           tie_flag = true;
           return 0;
        }
        return (diff1 < diff2)? a : b;
    }
    // Case 4: {b...referencePoint...a}
    diff1 = (long) referencePoint-b;
    diff2 = (long) a-referencePoint;
    if (diff1 == diff2)
    {
        tie_flag = true;
        return 0;
    }
    return (diff1 < diff2)? b : a;
}

A bit longer, but it never fails, whatever int's you feed. But if 'a' or 'b' are zero, and that zero is the nearest one to the reference point, the return value is indistinguisable from a tie, so this is why the auxiliary tie flag.

\$\endgroup\$
3
  • 2
    \$\begingroup\$ it looks to me like there is a lot of extraneous code here, can you please break this down and explain the need for each Case? looks like a lot of this is extra work that could be done simpler by one of the other solutions. And you never specify why it is simpler for the machine? what machine? \$\endgroup\$
    – Malachi
    Apr 14, 2015 at 18:39
  • \$\begingroup\$ If you follow the cases and the comments with positions of values (enclosed in brackets {...}), you'll find out that there's no repeated ones, things are done only once. \$\endgroup\$ Apr 14, 2015 at 18:54
  • \$\begingroup\$ And "Simpler for the machine" is only that: no calls, only comparisons mainly, and some substracts, all done in local variables (the stack). This function can be emplemented not only in C and C++, but even in assembler! So, What machine? Every machine. Try it in a benchmark against your favourite alternative, I bet mine is faster and safer (it doesn't fail with any three integers you choose). \$\endgroup\$ Apr 14, 2015 at 18:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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