patterncsharpMajor
Given two int values, return the one closer to 10
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
or
My solution:
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:
First of all, you can figure everything out if you know the distance between the values of
You have principle in this section right, but it can be simplified a bit if you set
Now, you just need to apply the same principle to the other values:
Here is our new method:
But,
Now, you can change your method as much as you want, and you only have the magic number
Edit:
Thanks to Eric Lippert, I have fixed two bugs. Eric found the bug that
Edit 2:
Eric Lippert says we should do the arithmetic in
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.