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

Divide decimal by range of fractions, and then convert the fraction that is closest to a whole number to a mixed fraction

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

Problem

My program will divide the decimal (\$30.0575\$) by a range of fractions (\$\frac{1}{1}, \frac{1}{2}, \frac{1}{4}, \dots, \frac{1}{48})\$ leaving me with list of improper fractions that look like this:

  • \$\frac{1}{1} * 30.0575\$



  • \$\frac{1}{2} * 60.115\$



  • \$\frac{1}{3} * 90.1725\$



  • \$\dots\$



  • \$\frac{1}{48} * 1442.76\$



My program will then "pick out" the improper fraction that contains the decimal that is closest to a whole number. It will finally convert the improper fraction that is closest to a whole number into a mixed fraction.

  • How can I make my program more readable?



  • Are there any "shortcuts" I could implement that would reduce the lines of code used?



  • How can I overall optimize my code so it completes the same task in a more efficient way, while still keeping readability?



```
private void FractionsForm_Load(object sender, EventArgs e)
{
var myDecimal = 30.0575;

var results = new List();

for (double i = 1; i ();

foreach (var result in results)
{
var roundedResult = Math.Round(result);
var remainder = roundedResult - result;
remainders.Add(Math.Abs(remainder));
}

var mostAccurateRemainder = remainders.Min();
var mostAccurateRemainderIndex = remainders.IndexOf(mostAccurateRemainder);
var fractionDenominator = mostAccurateRemainderIndex + 1;

Console.WriteLine("1/{0} * {1}", fractionDenominator, results[mostAccurateRemainderIndex]);

WriteLineSeparator();

var integerPart = Convert.ToInt32(myDecimal);
var integralPart = myDecimal - Math.Truncate(myDecimal);

Console.WriteLine("{0} {1}/{2}", integerPart, Convert.ToInt32(integralPart * fractionDenominator),
fractionDenominator);
}

private static void WriteLineSeparator()
{
Console.WriteLine(string.Empty);
Co

Solution

-
Trust the math.

var fraction = 1 / i;
        var result = myDecimal / fraction;


is a long way to say

var result = myDecimal * i;


-
Collecting results in the list is just a waste of space. You may maintain the most accurate candidate as you do compute results:

for (double i = 1; i < 49; i++)
    {
        var result = myDecimal * i;
        var roundedResult = Math.Round(result);
        var remainder = roundedResult - result;
        if (remainder < bestRemainder) {
            bestRemainder = remainder;
            bestDenominator = i;
        }
    }

Code Snippets

var fraction = 1 / i;
        var result = myDecimal / fraction;
var result = myDecimal * i;
for (double i = 1; i < 49; i++)
    {
        var result = myDecimal * i;
        var roundedResult = Math.Round(result);
        var remainder = roundedResult - result;
        if (remainder < bestRemainder) {
            bestRemainder = remainder;
            bestDenominator = i;
        }
    }

Context

StackExchange Code Review Q#134376, answer score: 4

Revisions (0)

No revisions yet.