HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMajor

Given two int values, return the one closer to 10

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thereturntwoonecloservaluesintgiven

Problem

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 = 10 || b >= 10) && b  calcA && Math.Abs(a)!= Math.Abs(b))
                {
                    return b;
                }
                else if ((calcA  calcB && Math.Abs(a) != Math.Abs(b)))
                {
                    return a;
                }
                else return Closer(a,b);
            }
        }
    }

Solution

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 = 10 || b >= 10) && b  calcA && Math.Abs(a)!= Math.Abs(b))
    {
        return b;
    }
    else if ((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;
}

Code Snippets

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);
}
int calcA = Math.Abs(a - 10);
int calcB = Math.Abs(b - 10);
if (calcA == calcB)
{
    return 0;
}
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"
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;
}

Context

StackExchange Code Review Q#86754, answer score: 30

Revisions (0)

No revisions yet.