Bugged my own If-Else statement

Question

So I'm a student that's learning Java. I am currently busy with an assignment, but I keep getting stuck with a certain bit of if-else logic.

My assignment requests that I should write a class file that takes a rational number (e.g. 72 / 14), display it, break it down its lowest form ( 36 / 7 in this case), display the rational number as a double value, and finally display the biggest common factor. What I'm getting stuck on is the fact the my code doesn't sift out the fact that for example, 72 / 13 is the smallest the rational number can become, but I got it so it won't crash, but in turn I traded it off for 72 / 14's value searching and such not working correctly.

My code:

public class QTwoAssiThree
{
    private int numerator;
    private int denominator;

    public void writeOutput()
    {
        if (getGCD(numerator, denominator) >= 2)
        {
            System.out.println(toString() + " is the simplified rational number.");
            System.out.println(getValue() + " is the value of the two rational numbers as one number.");
            System.out.println((getGCD(numerator, denominator) * 2) + " is the largest common factor.");
        }
        else if (getGCD(numerator, denominator) == 1)
        {
            System.out.println(getValue() + " is the value of the two rational numbers as one number.");
            System.exit(0);
        }       
    }
    public QTwoAssiThree()
    {
        numerator = 0;
        denominator = 0;
    }
    public QTwoAssiThree(int initialNum, int initialDenom)
    {
        if (denominator % 2 == 0)
        { 
            numerator = initialNum;
            if (initialDenom > 0)
            {
                denominator = initialDenom;
                System.out.println(toString() + " is the rational number entered.");
                simplify(numerator, denominator);
            }
            else
            {
                System.out.println("Invalid value added as the denominator! Only positive integers should be used.");
                System.exit(0);
            }
        }
        else if (denominator % 2 == 1)
        {
            System.out.println(toString() + " is what was entered, and is already in its simplest form.");
        }   
    }
    private void simplify(int newNum, int newDenom)
    {
        numerator = newNum / getGCD(newNum, newDenom);
        denominator = newDenom / getGCD(newNum, newDenom);
    }
    private static int getGCD(int x, int y)
    {
        int difference = x % y;

        return difference;
    }
    private double getValue()
    {
        double doubleValue = (double)numerator / (double)denominator;

        return doubleValue;
    }
    public String toString()
    {
        String fraction = Integer.toString(numerator) + "/" + Integer.toString(denominator);

        return fraction;
    }
}

Now, please ignore the fact that I'm still somewhat low on the knowledgeable Java programmer scale. I would appreciate any pointers on this, as I got stuck and can't really figure out how to set it up. There's two places where I used if-else to see if it would work, the first being in: public void writeOutput() and in the public QTwoAssiThree(int initialNum, int initialDenom) constructor. Initially there weren't any if-else statements there, code did work though, but not for uneven integers.

public class QTwoAssiThreeDemo
{
    public static void main(String[] args)
    {
        QTwoAssiThree rationalNumbers = new QTwoAssiThree(72, 14);

        rationalNumbers.writeOutput();
    }
}

is used to demonstrate the whole class.

Basically what my outputs should look like is as follow:

    72/14 is the rational number entered.
    36/7 is the simplified rational number.
    5.142857143 is the value of the two rational numbers as one number.
    2 is the largest common factor.

and

    72/13 is what was entered,  and is already in its simplest form.
    5.538461538 is the value of the two rational numbers as one number.

The code is unoptimized and looks ugly, I know, but any help would be appreciated, I will do a run through it all to make it look a bit nicer before I submit the assignment.

First, you´ve implemented *wrong* GCD algorithm (`getGCD`): what if you're given `14` and `72` instead of `72` and `14`? Another counter example is `4` and `2` — Dmitry Bychenko, May 20 '15 at 08:08
I think there is problem with `getGCD()` can you say what it has to do? — theRoot, May 20 '15 at 08:09
your problem is in `getGCD()` as well as you are checking GCD again in `writeOutput()` on simplest rational form — Prashant, May 20 '15 at 08:12
why can't you use java's BigInteger [GCD](https://docs.oracle.com/javase/8/docs/api/java/math/BigInteger.html#gcd-java.math.BigInteger-) — theRoot, May 20 '15 at 08:13
@DmitryBychenko I noticed that also just now that it would cause an issue if the numerator is smaller than the denominator. At least I have an idea where the issue lies now. — Tower's Vault, May 20 '15 at 08:19

score 1 · Accepted Answer · edited May 23 '17 at 12:14

code that need to be altered (IMO) :

Constructor must be used only to initialize the object
Altered GCD function following the refered link
You can add the simplify function inside constructor (since it re-initializes the values )
Use try and catch blocks to check if they are valid inputs else throw Exceptions

Altered Code :

public class QTwoAssiThree {
    private int numerator;
    private int denominator;

    public void writeOutput() {

        System.out.println(toString() + " is the rational number entered.");
        if (simplify(numerator, denominator)>1)
            System.out.println(toString()+ " is the simplified rational number.");
        else
            System.out.println(toString()+ " is what was entered, and is already in its simplest form.");
        System.out .println(getValue() +" is the value of the two rational numbers as one number.");
    }

    public QTwoAssiThree() {
        numerator = 0;
        denominator = 0;
    }

    public QTwoAssiThree(int initialNum, int initialDenom) {

        numerator = initialNum;
        denominator = initialDenom;
        if (initialDenom <= 0) {
            System.out
                    .println("Invalid value added as the denominator! Only positive integers should be used.");
            System.exit(0);
        }
    }

    private int simplify(int newNum, int newDenom) {
        int gcd = getGCD(newNum, newDenom);
        numerator = newNum / gcd;
        denominator = newDenom / gcd;
        return gcd;
    }

    private static int getGCD(int x, int y) {

        if (y == 0) {
            return x;
        }
        return getGCD(y, x % y);
    }

    private double getValue() {
        double doubleValue = (double) numerator / (double) denominator;

        return doubleValue;
    }

    public String toString() {
        String fraction = Integer.toString(numerator) + "/"
                + Integer.toString(denominator);

        return fraction;
    }
}

Oh man, I just started to rewrite basically everything, I made a private int in the simplify method as well. Thanks for helping, didn't think it was this simple. — Tower's Vault, May 20 '15 at 09:06

Bugged my own If-Else statement

1 Answers1