Hello and welcome to our community! Is this your first visit?
Enjoy an ad free experience by logging in. Not a member yet? Register.

# Thread: fraction class is killing me

1. ## fraction class is killing me

so for my 102 class i was told to make a fraction class and i was given a test driver to make sure i did everything right. however, it seems that all my add, subtract, multiply, and divide fractions have some type of error in them or else i'm messing up when i try to simplify the fraction. here's the code for my Fraction class, if you want to see the test driver, i have that available as well. thanks

Code:
```/**
* This class will store a fraction in reduced form and
* allow for the user to add, subtract, multiply, or divide
* the fraction.
*
* @author Bryan Chan
* @version Program 1
* @version CPE102-9
* @version Winter 2009
*/

public class Fraction
{
private int num, denom;
private double numValue;

//constructors
public Fraction()
{
num = 0;
denom = 1;
}

public Fraction( int numerator )
{
num = numerator;
denom = 1;
}

public Fraction( int numerator, int denominator )
{
num = numerator / gcd( numerator, denominator );
denom = denominator / gcd( numerator, denominator );

if( num == 0 )
{
denom = 1;
}

if( num == denom )
{
num = 1;
denom = 1;
}

if( denom <= 0 )
{
throw new IllegalArgumentException();
}
}

//methods
public Fraction add( Fraction other )
{
if( denom == other.getDenominator() )
{
num += other.getNumerator();
}

else
{
num = ( num * other.getNumerator() ) + ( other.getNumerator() * denom );
denom *= other.getDenominator();
}

System.out.print( num + "/" + denom + "\n" );

return new Fraction( num, denom );
}

public Fraction div( Fraction other )
{
num = num * other.getDenominator();
denom = denom * other.getNumerator();

return new Fraction( num, denom );
}

public boolean equals( Fraction other )
{
if( ( num / denom ) == ( other.getNumerator() / other.getDenominator() ) )
{
return true;
}
else
{
return false;
}
}

public int getDenominator()
{
return denom;
}

public int getNumerator()
{
return num;
}

public Fraction mul( Fraction other )
{
num = num * other.getNumerator();
denom = denom * other.getDenominator();

System.out.print( num + "/" + denom + "\n" );

return new Fraction( num, denom );
}

public Fraction sub( Fraction other )
{
if( denom == other.getDenominator() )
{
num -= other.getNumerator();
}
else
{
num = ( num * other.getNumerator() ) + ( other.getNumerator() * denom );
denom *= other.getDenominator();
}

return new Fraction( num, denom );
}

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

public String toString()
{
if( denom == 1 )
{
return "" + num;
}
else
{
return num + "/" + denom;
}
}

public double value()
{
return (double)num / denom;
}

}```

I haven't gone through most of this code, but it does look ok, and nicely formatted too.
Just looking at the multiply (since its easiest), and yeah that looks fine.
I'm thinking its in the lowest common denominator check, but I'm trying to see where you call that >.<
I'll take a peek when I get home from work.

• hey, thanks for the quick reply! the test driver is coded below.

Code:
```import java.util.Scanner;
import java.lang.reflect.*;

public class P1TestDriver
{
private static final String RESULTS_FOR = "Results for Program 1";
private static final double EPSILON = 0.000000000000001;

public static void main(String[] args)
{
boolean pass = true;

pass &= testArchitecture();

if (pass)
{
pass &= testDefaultConstructor();
pass &= testNumeratorConstructor();
pass &= testNumeratorDenominatorConstructor();
pass &= testToString();
pass &= testEquals();
pass &= testSub();
pass &= testMul();
pass &= testDiv();
}

printResults(pass);

// Added for to support robust script checking
if (!pass)
{
System.exit(-1);
}
}

// Test default constructor
private static boolean testDefaultConstructor()
{
boolean pass = true;
System.out.println("   Testing default constructor - Fraction()...");

pass &= test(new Fraction(), 0, 1);

return pass;
}

// Test numerator consructor
private static boolean testNumeratorConstructor()
{
boolean pass = true;
System.out.println("   Testing constructor - Fraction(int)...");
String msg = "      FAILED: ";

pass &= test(new Fraction(864), 864, 1);
pass &= test(new Fraction(-368), -368, 1);
pass &= test(new Fraction(0), 0, 1);

return pass;
}

// Test numerator/denominator consructor
private static boolean testNumeratorDenominatorConstructor()
{
boolean pass = true;
System.out.println("   Testing constructor - Fraction(int, int)...");

pass &= test(new Fraction(1, 2), 1, 2);
pass &= test(new Fraction(2, 1), 2, 1);

pass &= test(new Fraction(4, 8), 1, 2);
pass &= test(new Fraction(8, 4), 2, 1);

pass &= test(new Fraction(15, 75), 1, 5);
pass &= test(new Fraction(75, 15), 5, 1);

pass &= test(new Fraction(-6, 9), -2, 3);
pass &= test(new Fraction(0, 3), 0, 1);
pass &= test(new Fraction(255255, 1616615), 3, 19);

boolean caught = false;
try
{
new Fraction(9, -6);
}
catch(IllegalArgumentException e)
{
caught = true;
}
pass &= test(caught, "      FAILED: IllegalArgumentExcpetion not thrown");

caught = false;
try
{
new Fraction(3, 0);
}
catch(IllegalArgumentException e)
{
caught = true;
}
pass &= test(caught, "      FAILED: IllegalArgumentExcpetion not thrown");

return pass;
}

// Test toString
private static boolean testToString()
{
boolean pass = true;
String msg = "      FAILED: ";
System.out.println("   Testing toString()...");

Fraction f = new Fraction(5, 6);
pass &= test(f.toString().equals("5/6"), msg +
"toString() is " + f.toString() + ", expected 5/6");

f = new Fraction(49, 56);
pass &= test(f.toString().equals("7/8"), msg +
"toString() is " + f.toString() + ", expected 7/8");

f = new Fraction(13, 5);
pass &= test(f.toString().equals("13/5"), msg +
"toString() is " + f.toString() + ", expected 13/5");

return pass;
}

// Test equals
private static boolean testEquals()
{
boolean pass = true;
String msg = "      FAILED: ";
System.out.println("   Testing equals(Fraction)...");

Fraction f1 = new Fraction(5, 6);
Fraction f2 = new Fraction(10, 12);
Fraction f3 = new Fraction(1, 2);

pass &= test(f1.equals(f1), msg + "equals() is true, expected false");
pass &= test(f1.equals(f2), msg + "equals() is true, expected false");
pass &= test(!f2.equals(f3), msg + "equals() is false, expected true");

return pass;
}

{
boolean pass = true;
String msg = "      FAILED: ";

Fraction f1 = new Fraction(5, 6);
Fraction f2 = new Fraction(25, 30);
Fraction f3 = new Fraction(1, 2);
Fraction f4 = new Fraction(5, 10);

pass &= test(f1, 5, 6);

pass &= test(f1, 5, 6);
pass &= test(f2, 5, 6);

pass &= test(f1, 5, 6);
pass &= test(f3, 1, 2);

pass &= test(f1, 5, 6);
pass &= test(f4, 1, 2);

pass &= test(f3, 1, 2);
pass &= test(f4, 1, 2);

return pass;
}

// Test sub
private static boolean testSub()
{
boolean pass = true;
String msg = "      FAILED: ";
System.out.println("   Testing sub(Fraction)...");

Fraction f1 = new Fraction(5, 6);
Fraction f2 = new Fraction(25, 30);
Fraction f3 = new Fraction(1, 2);
Fraction f4 = new Fraction(5, 10);

pass &= test(f1.sub(f1), 0, 1);
pass &= test(f1, 5, 6);

pass &= test(f1.sub(f2), 0, 1);
pass &= test(f1, 5, 6);
pass &= test(f2, 5, 6);

pass &= test(f1.sub(f3), 1, 3);
pass &= test(f1, 5, 6);
pass &= test(f3, 1, 2);

pass &= test(f1.sub(f4), 1, 3);
pass &= test(f1, 5, 6);
pass &= test(f4, 1, 2);

pass &= test(f3.sub(f4), 0, 1);
pass &= test(f3, 1, 2);
pass &= test(f4, 1, 2);

return pass;
}

// Test mul
private static boolean testMul()
{
boolean pass = true;
String msg = "      FAILED: ";
System.out.println("   Testing mul(Fraction)...");

Fraction f1 = new Fraction(5, 6);
Fraction f2 = new Fraction(25, 30);
Fraction f3 = new Fraction(1, 2);
Fraction f4 = new Fraction(5, 10);
Fraction f5 = new Fraction(1, 6);
Fraction f6 = new Fraction(3, 1);

pass &= test(f1.mul(f1), 25, 36);
pass &= test(f1, 5, 6);

pass &= test(f1.mul(f2), 25, 36);
pass &= test(f1, 5, 6);
pass &= test(f2, 5, 6);

pass &= test(f1.mul(f3), 5, 12);
pass &= test(f1, 5, 6);
pass &= test(f3, 1, 2);

pass &= test(f1.mul(f4), 5, 12);
pass &= test(f1, 5, 6);
pass &= test(f4, 1, 2);

pass &= test(f3.mul(f4), 1, 4);
pass &= test(f3, 1, 2);
pass &= test(f4, 1, 2);

pass &= test(f5.mul(f6), 1, 2);
pass &= test(f5, 1, 6);
pass &= test(f6, 3, 1);

return pass;
}

// Test div
private static boolean testDiv()
{
boolean pass = true;
String msg = "      FAILED: ";
System.out.println("   Testing div(Fraction)...");

Fraction f1 = new Fraction(5, 6);
Fraction f2 = new Fraction(25, 30);
Fraction f3 = new Fraction(1, 2);
Fraction f4 = new Fraction(5, 10);

pass &= test(f1.div(f1), 1, 1);
pass &= test(f1, 5, 6);

pass &= test(f1.div(f2), 1, 1);
pass &= test(f1, 5, 6);
pass &= test(f2, 5, 6);

pass &= test(f1.div(f3), 5, 3);
pass &= test(f1, 5, 6);
pass &= test(f3, 1, 2);

pass &= test(f1.div(f4), 5, 3);
pass &= test(f1, 5, 6);
pass &= test(f4, 1, 2);

pass &= test(f3.div(f4), 1, 1);
pass &= test(f3, 1, 2);
pass &= test(f4, 1, 2);

return pass;
}

private static boolean testArchitecture()
{
boolean pass = true;
String msg = "      FAILED: ";
Fraction f;
Class cl;
Class[] temp;

System.out.println("   Testing Fraction architecture...");

f = new Fraction();

cl = f.getClass();

int cnt = cl.getConstructors().length;
pass &= test(cnt == 3, msg + cnt + " constructors, expected 3");

cnt = countModifiers(cl.getDeclaredMethods(), Modifier.PUBLIC);
pass &= test(cnt == 9, msg + cnt + " public methods, expected 9");

cnt = cl.getFields().length;
pass &= test(cnt == 0, msg + cnt + " public fields, expected 0");

cnt = countModifiers(cl.getDeclaredFields(), Modifier.PROTECTED);
pass &= test(cnt == 0, msg + cnt + " protected fields, expected 0");

cnt = countModifiers(cl.getDeclaredFields(), Modifier.PRIVATE);
pass &= test(cnt <= 3, msg + cnt
+ " private fields, expected 3 or fewer");

cnt = cl.getDeclaredFields().length
- countModifiers(cl.getDeclaredFields(), Modifier.PRIVATE)
- countModifiers(cl.getDeclaredFields(), Modifier.PROTECTED)
- countModifiers(cl.getDeclaredFields(), Modifier.PUBLIC);
pass &= test(cnt == 0, msg + cnt + " package fields, expected 0");

return pass;
}

{
if (args.length == 1)
{
System.out.println(args[0]);
}

System.out.println(RESULTS_FOR + "\n");
}

private static void printResults(boolean pass)
{
String msg;

if(pass)
{
msg = "\nCongratulations, you passed all the tests!\n\n"
+ "correct solution and any deductions for the quality of your\n"
+ "implementation.  Quality is based on, but not limited to,\n"
+ "coding style, documentation requirements, compiler warnings,\n"
+ "and the efficiency and elegance of your code.\n";
}
else
{
msg = "\nNot done yet - you failed one or more tests!\n";
}

System.out.print(msg);
}

private static boolean approx(double a, double b)
{
return Math.abs(a - b) < EPSILON;
}

private static boolean test(boolean pass, String msg)
{
if(!pass)
{
System.out.println(msg);
}

return pass;
}

private static boolean test(Fraction f, int n, int d)
{
boolean pass = true;
String msg = "      FAILED: ";

pass &= test(f.getNumerator() == n, msg +
"Numerator is " + f.getNumerator() + ", expected " + n);

pass &= test(f.getDenominator() == d, msg +
"Denominator is " + f.getDenominator() + ", expected " + d);

pass &= test(approx(f.value(), (double)n/d), msg +
"Value is " + f.value() + ", expected " + (double)n/d);

return pass;
}

private static int countModifiers(Field[] fields, int modifier)
{
int count = 0;

for (Field f : fields)
{
if (f.getModifiers() == modifier)
{
count++;
}
}

return count;
}

private static int countModifiers(Method[] methods, int modifier)
{
int count = 0;

for (Method m : methods)
{
if (m.getModifiers() == modifier)
{
count++;
}
}

return count;
}

}```

• Mkay, I can't believe how close this is.
A few initial problems. First is you're equals method. num / denom is integer / integer division, which will result in an integer not a double. Cast them to doubles to produce the double result, or be lazy like me
PHP Code:
``` public boolean equals(Fraction other) {     return this.num == other.num && this.denom == other.denom; }  ```
This (or the casting) should produce the correct equals value.

Next up, the second checks. These methods really feel more like static ones to me, and should probably be `public static Fraction add(Fraction f1, Fraction f2)`. The second checks confirm the original data in the objects. So, each of you're add, subtract, multiply and division methods need to create local variables. I'm lazy so I just reused:
PHP Code:
``` public Fraction add(Fraction other) {     int num = this.num;     int denom = this.denom;     .... }  ```
That will fix all except the addition and subtraction errors. You're simplification technique is incorrect. Though using more if/else statements could optimize this a little more (to save on the modulus checking later), I would just alter what you have.
`num = ( num * other.getNumerator() ) + ( other.getNumerator() * denom );`

Should be:
`num = (num * other.getDenominator()) + (other.getNumerator() * denom);`

And in subtraction:
`num = ( num * other.getNumerator() ) + ( other.getNumerator() * denom );`

Should be:
`num = (num * other.getDenominator()) - (other.getNumerator() * denom);`

So instead of numerator * other numerator, they should be numerator * other denominator, and opposite the other way around. The subtraction method was trying to add >.<

And finally, remove the numValue variable, its unused. Don't forget to comment all of you're code (according to the test pass, it says thats one of the criteria) and remove the system.out.println calls!

Man, that was the most amount of debugging work I've ever done with the least amount of code changing >.<

• It looks as though the test driver uses negative arguments and your code throws an exception for that? It's easy enough to carry the sign around... I remember doing the Fraction class in C++ for high school, but I don't remember if I used a separate class variable or stored it in the numerator. (I think the latter.)

There is also the issue of maintaining most simplified form; I noticed that you do it in the constructor. That's a perfectly fine way -- another decent way is to guarantee that the arithmetical operations return reduced fractions (this would require use of lcm()). If you're working with HUGE numerators and denominators, this would prevent overflow quicker than the constructor method would. (Admittedly, however, for something as academic as this, it may not be worth the effort.)

Lastly, if the equality check of (num1 == num2 && denom1 == denom2) only works if you are absolutely sure of the canonical form of both fractions (reduced, and negative sign in the numerator). A more robust check is to note that a/b = c/d implies a*d = b*c. So you can check for equality by (num1*denom2 == num2*denom1). If you're sure your fractions are always canonical, however, then obviously Fou-Lou's check will be better since it doesn't involve multiplication. Good luck.

• wow thanks a lot...everything you suggested worked perfectly. i know i've seen the "this" keyword around a lot but i havent actually learned what it does, but i think im going to google it right now, seems pretty helpful hehe ^^. Once again, thanks a buuunch

• Originally Posted by uniquity
wow thanks a lot...everything you suggested worked perfectly. i know i've seen the "this" keyword around a lot but i havent actually learned what it does, but i think im going to google it right now, seems pretty helpful hehe ^^. Once again, thanks a buuunch
This simply refers to this objects instance, the collection on 'this' object. You don't see it as often in languages like Java since its not required unless you're using variable masking (which I use a lot of). So the fix was to redefine the value of num and denom and we assigned a value of this.num and this.denom. At this point, this is also not required since num and denom are class variables. Later in the code, if you wanted to refer only to the value of the class objects variable, you'd have to use this. since the variable was redefined.
Languages like PHP require the \$this keyword when accessing their objects, which is probably why I tend to use this.x even when I don't need to.

• ## Users who have thanked Fou-Lu for this post:

uniquity (01-15-2009)

•

#### Posting Permissions

• You may not post new threads
• You may not post replies
• You may not post attachments
• You may not edit your posts
•