Converting fractions to mixed numbers

Question

I've been making a program for an assignment that converts fractions to mixed numbers, and I'm done, but I was wondering if I could simplify it in any way, since it seems quite complicated right now.

Also, keep in mind that I am not allowed to use any Math or String methods to do this assignment, and I can only use +, -, *, and / as arithmetic operators. I also cannot use % or a modulus function.

I realize this is an assignment, and it's fine if you choose not to give me an answer. In fact, I'd actually prefer to have some tips or hints to help figure out how to solve it, rather than just telling me the answer.

Also, the 'c.print' is basically a print statement. I'm importing/using a different custom class, which is why it is different from the normal print statement.

    int numerator = 0; //can change to any value
int denominator = -2; //can change to any value
public int[] reducedFraction (int numerator, int denominator)
{
    int GCD = numerator;
    int tempN = numerator;
    int tempD = denominator;
    int[] fractionParts = {0, numerator, denominator};

    if (numerator == 0) //if the numerator is 0
    {
        return fractionParts;
    }

    //Making sure the program works with negative values by temporarily converting GCD and fractionParts[2] to positive values
    GCD = (numerator < 0) ? -numerator:
    numerator;

    tempD = (denominator < 0) ? -denominator:
    denominator;

    //Finding the GCD of the numerator and denominator
    while (GCD != tempD)
    {
        if (GCD > tempD)
            GCD -= tempD;
        else
            tempD -= GCD;
    }

    //Simplfying numerator and denominator
    fractionParts [1] /= GCD;
    fractionParts [2] /= GCD;

    //Temporarily making the numerator and denominator positive
    tempN = (fractionParts [1] < 0) ? -fractionParts [1]:
    fractionParts [1];

    tempD = (fractionParts [2] < 0) ? -fractionParts [2]:
    fractionParts [2];

    //if the numerator is not greater than the denominator return the simplified fraction.
    if (tempN < tempD)
    {
        return fractionParts;
    }

    //Finding value of whole number, numerator and possibly converting denominator to original sign
    fractionParts [0] = fractionParts [1] / fractionParts [2];
    fractionParts [1] = fractionParts [1] - (fractionParts [1] / fractionParts [2]) * fractionParts [2];

    if (fractionParts [1] < 0 && fractionParts [2] < 0) //if the numerator and denominator are both negative
    {
        fractionParts [1] = (fractionParts [1] < 0) ? -fractionParts [1]:
        fractionParts [1];

        fractionParts [2] = (fractionParts [2] < 0) ? -fractionParts [2]:
        fractionParts [2];
    }

    return fractionParts;
}


public void display ()
{
    if (reducedFraction (numerator, denominator) [1] == 0)
        c.print (reducedFraction (numerator, denominator) [0]);

    else if (reducedFraction (numerator, denominator) [0] == 0)
        c.print (reducedFraction (numerator, denominator) [1] + "/" + reducedFraction (numerator, denominator) [2]);

    else
        c.print (reducedFraction (numerator, denominator) [0] + " " + reducedFraction (numerator, denominator) [1] + "/" + reducedFraction (numerator, denominator) [2]);
}

Answer 1

First, a couple of tips:

• Using an int[] to represent the result is a poor choice. It would be better to create a class for this, for example called MixedNumber. Then, instead of accessing the elements by indexes, you can access them by field name. It could also have an appropriate toString method to print itself

• The handling of negative numbers is very poor. You could determine the sign of the number in one place early on, use it to multiply the integral part and that's it. That would get rid of many confusing redundant logic. The current code also has a bug related to it: for the input (3, -2) it gives back -1 1/-2 instead of -1 1/2

• An elegant implementation of the gcd algorithm is this recursive function: return b == 0 ? a : gcd(b, a % b);

• The printing of the number can be cleaner and simpler using String.format

I recommend to rewrite the function in terms of this class:

class MixedNumber {
    private final int integral;
    private final int numerator;
    private final int denominator;

    private MixedNumber(int integral, int numerator, int denominator) {
        this.integral = integral;
        this.numerator = numerator;
        this.denominator = denominator;
    }

    private static int gcd(int a, int b) {
        return b == 0 ? a : gcd(b, a % b);
    }

    static MixedNumber fromFraction(int numerator, int denominator) {
        // TODO: simplified main implementation comes here
    }

    @Override
    public String toString() {
        if (numerator == 0) {
            return "" + integral;
        } else if (integral == 0) {
            return String.format("%d/%d", numerator, denominator);
        } else {
            return String.format("%d %d/%d", integral, numerator, denominator);
        }
    }
}

Rather than printing the output, use unit tests to verify. After you finished the implementation above, with the bug that I mentioned fixed, these unit tests should pass:

@Test
public void testZeroNumerator() {
    assertEquals("0", MixedNumber.fromFraction(0, -2).toString());
}

@Test
public void test_minus3_over_2() {
    assertEquals("-1 1/2", MixedNumber.fromFraction(-3, 2).toString());
}

@Test
public void test_3_over_minus2() {
    assertEquals("-1 1/2", MixedNumber.fromFraction(3, -2).toString());
}

@Test
public void test_minus3_over_minus2() {
    assertEquals("1 1/2", MixedNumber.fromFraction(-3, -2).toString());
}

@Test
public void test_123_over_82() {
    assertEquals("1 1/2", MixedNumber.fromFraction(123, 82).toString());
}

@Test
public void test_82_over_123() {
    assertEquals("2/3", MixedNumber.fromFraction(82, 123).toString());
}

Whatever IDE you use, there is certainly an easy way to add a JUnit4 test class and run it. Give it a try!

Answer 2

Here are a few suggestions for changes:

(1) Instead of array name with 3 elements use 3 variable names (whole, numerator, denominator). fractionParts array name is less descriptive.

(2) Use modulo operator %. Based on (1) and (2), code segment

//Finding value of whole number, numerator and possibly converting denominator to original sign
fractionParts [0] = fractionParts [1] / fractionParts [2];
fractionParts [1] = fractionParts [1] - (fractionParts [1] / fractionParts [2]) * fractionParts [2];

becomes shorter and easier to read. Example of how % operator fits in is in the comments.

whole = numerator / denominator;       // e.g. 0 10/3:  n=10 / d=3 => w=3
numerator %= denominator;              // e.g.          n=10 % d=3 => n=1     0 10/3 becomes 3 1/3

(3) Create and use gcd method. Main code will become a little shorter and it will be easier to use the GCD code elsewhere.

private int gcd(int a, int b) {
    if (b == 0) return a;
    return gcd(b, a%b);
}