Questions, bugs, and feature requests about bigfraction HOT 7 CLOSED

Thanks for the feedback! I'll try to look at them sometime over the next week. Here are a few quick comments:

Looks like the Javadoc errors should be easy to resolve. Not sure if I will bother fixing the warnings--it gets really tedious to document every @return and @param when the main method description already covers that.

.toString(): This object is a fraction, so I wanted the string to always be a fraction. Generally built-in toString() methods are more for debugging than presentation. That said, I may add an optional boolean parameter to indicate whether to display whole numbers as integers.

Constructors: There are a few reasons I did it like this:

• At one point I did have multiple constructors, but there was lots of repeated code among the constructors. Mainly because in Java if one constructor delegates to another constructor, it must be the first line of the constructor.
• The valueOf() approach is also used by BigInteger and BigDecimal classes, which served as my "inspiration". While those classes also have constructors, their javadoc states that valueOf() is the preferred means of creating those objects.
• Using valueOf() also opens the door to possible future enhancement where previously-computed fractions are cached for faster re-use.
• A lot of the math relies on a guarantee that the fraction is already in lowest terms. The private constructor takes boolean reduced variable. I use this internally to avoid the potentially-costly step of finding the GCD in order to reduce the fraction, if I happen to know that it is already reduced. But I can't trust that value from external code.

As for the remainder methods- I've never included those because it never made sense to me with fractions. For example, (5/6) / (3/4) = (10/9). What is the remainder? Is the answer 1 with remainder (1/9)? If that's the case, you can just divide, store the result in a variable, then call .getFractionPart() to get the remainder. I will look at the methods you posted later this week when I get some time, but I am unclear of the kind of problems that would need a remainder for fractional division, or what fractional remainder even means.

from bigfraction.

Interesting! I think the boolean parameter for toString is an excellent idea. I can see how toString can be used in debugging to ensure proper output, though as I am the data type user, and not the creator, I use it as a human-readable way of outputting the data type (to the console, for example, in programs that take rational inputs). The boolean param is probably the best compromise, though still in question would be the default behaviour.

For the constructors, you could simply have ones like these:

  public BigFraction(Number num, Number den){
BigFraction f=BigFraction.valueOf(num,den);
numerator=f.getNumerator();
denominator=f.getDenominator();
}
public BigFraction(Number n){
BigFraction f=BigFraction.valueOf(n);
numerator=f.getNumerator();
denominator=f.getDenominator();
}
public BigFraction(String s){
BigFraction f=BigFraction.valueOf(s);
numerator=f.getNumerator();
denominator=f.getDenominator();
}

Of course, they would mainly be for consistency, as BigInteger and BigDecimal both allow constructors or the valueOf approach. I still think it's worthwhile, and it's clearly very easy to code.

Mathematically, the remainder of x/y is defined as x-floor(x/y)*y (Java uses x-truncate(x/y)*y for the % operator). You can read more on the definition of the modulo operation here. There's no reason that x and y have to be integers, which is why the modulo operation works for fractions, and why BigDecimal includes modulo. The best way to illustrate why this makes sense is to use a problem:

Imagine you want to split 5/4 of a pizza amongst your friends, and you want to give each friend 1/2 a pizza. How much pizza will you have left over? This is essentially a Euclidian Division problem, except the dividend and divisor aren't integers. In this case, the Euclidian quotient is 2, and the remainder is 1/4. This makes sense, as you can split 5/4 of a pizza in 1/2 pizza units among 2 friends, and you will have exactly 1/4 left. In your original example, (5/6)mod(3/4) is 1/12, not 1/9.

My methods essentially calculate the exact value of x-floor(x/y)*y , with alternate methods that do x-truncate(x/y)*y and x-floor(x/abs(y))*abs(y) , to cover the different definitions of modulo discussed in the former link. Again, default behaviour is up for questioning, though I would imagine x-truncate(x/y)*y because that's how BigInteger and BigDecimal use it.

Anyway, thanks for your quick reply! If there's a better way to communicate let me know.

from bigfraction.

Thanks again for the feedback. Latest commit should resolve all of your comments. Javadoc still gives warnings but should not give errors any more. I may go back and try to further optimize some of the methods I added.

Please let me know if you still see any issues!

from bigfraction.

Thanks! I'll have to look at the remainder methods. I noticed it's not included for the java 7 branch yet. Anyway, I still get this error when making the javadoc (in the java 8 version; java 7 version never had javadoc issues):

*** Error ***
-------------
File: /Users/elygolden/Downloads/BigFraction-master/src/com/github/kiprobinson/util/BigFraction.java  [line: 647]
Error: error: no summary or caption for table

UPDATE: While I'm not as skilled as you are, I think You can optimize the remainder really well by doing the "gruntwork" in the divideToIntegralValue(DivisionMode dm) method instead of the divideAndRemainderImpl method; removing the need for it entirely. Then you can take advantage of remainder=dividend-(divisor*integralQuotient) like so:

public BigFraction remainder(Number n,DivisionMode dm){
 return this.subtract(valueOf(this.divideToIntegralValue(n,dm)).multiply(n));
}

Then your divideAndRemainder method could look like this:

public Number[] divideAndRemainder(Number n,DivisionMode dm){
Number returnParts=new Number[2];
returnParts[0]=this.divideToIntegralValue(n,dm);
returnParts[1]=this.subtract(valueOf(returnParts[0]).multiply(n));
return returnParts;
}

Also, do you think it's a good idea to add String versions of the remainder methods in case people don't want to import the enum DivisionMode?

from bigfraction.

I've fixed javadoc (again). I've also merged latest changes into java7 branch, which I hadn't done in a while.

Hopefully I'll get a chance to do some benchmarking on different ways of computing integer division and fractional remainder to figure out which method is the best. What I have is mainly a result of me figuring out a bunch of problems on paper, then putting those same problems into unit tests, then banging away until all my unit tests were passing. :)

As you can probably tell by the frequency of updates, I maintain this project in spare time, which I don't have a lot of lately...

from bigfraction.

finally the javadoc is working ^^ Thanks so much!

Whenever you get the chance you can look at my optimizations. Again, you're a much more skilled coder than I am, so you'd have to do the testing on your way vs my way vs some other ways to find the more efficient way.

Again, thank you for maintaining this project, even in your spare time! Of all the fraction classes I have seen this one is probably the most efficient and robust.

from bigfraction.

FYI I started doing some performance testing. I tried your suggestion of computing remainder using (a - b*q) and it is a little slower than the code I had written. Here are the times for three different versions when I tested on my laptop:

// 2.6s
BigFraction rFract = new BigFraction(r.multiply(b.numerator), b.denominator.multiply(den), Reduced.NO);

// 3.2s
BigFraction rFract = a.subtract(b.multiply(q));

// 2.9s
// =>  a - b*q = a.n/a.d - b.n*q/b.d = (a.n*b.d - b.n*a.d*q)/(a.d*b.d)
BigFraction rFract = new BigFraction(a.numerator.multiply(b.denominator).subtract(b.numerator.multiply(a.denominator).multiply(q)), a.denominator.multiply(b.denominator), Reduced.NO);

That was the best time to do about 251k quotientAndRemainder() calls.

I'm going to look into other areas to optimize, and I'll probably look at your original versions of the functions. For one thing, if I am getting only the integral quotient or only the remainder, I can probably do better because right now I calculate both and just throw out one of them. I can also do some optimization if you are doing truncated division mode, since that's what underlying BigInteger is already doing.

For my test, basically I pick two prime denominators (499 and 503), and create all fractions in (0,5] with that denominator, then I loop through all the combinations of those (499*503=250997). I repeat that test 5 times--usually the first time is about 5ms slower, probably due to JIT optimization, but the other four iterations are pretty consistent. I run a few times and record the approximate best time.

Here is the performance test code that I used, if you're curious:

import com.github.kiprobinson.util.BigFraction;

public class PerfTest
{
  public static void main(String[] args)
  {
    final int DEN1 = 499;
    final int DEN2 = 503;
    final int ITERATIONS = 5;

    BigFraction[] vals1 = new BigFraction[DEN1*5 - 1];
    for(int i = 0; i < vals1.length; i++)
      vals1[i] = BigFraction.valueOf(i+1, DEN1);

    BigFraction[] vals2 = new BigFraction[DEN2*5 - 1];
    for(int i = 0; i < vals2.length; i++)
      vals2[i] = BigFraction.valueOf(i+1, DEN2);

    for(int i = 0; i < ITERATIONS; i++) {
      Timer timer = new Timer();
      for(BigFraction a : vals1) {
        for(BigFraction b : vals2) {
          BigFraction.quotientAndRemainder(a, b);
        }
      }
      timer.stop();
      System.out.println(timer);
    }
  }
}

class Timer
{
  private long startTime;
  private long endTime = 0L;
  private boolean isRunning = true;

  public Timer() {
    startTime = System.currentTimeMillis();
  }

  public Timer stop() {
    endTime = System.currentTimeMillis();
    isRunning = false;
    return this;
  }

  public String toString() {
    if(isRunning)
      endTime = System.currentTimeMillis();
    long elapsed = endTime - startTime;
    long tmp = elapsed;

    long elMS = tmp % 1000L;
    tmp /= 1000L;
    long elS = tmp % 60L;
    tmp /= 60L;
    long elM = tmp % 60L;
    tmp /= 60L;
    long elH = tmp;

    return String.format("%02d:%02d:%02d.%03d (%d ms)", elH, elM, elS, elMS, elapsed);
  }
}

from bigfraction.