0
\$\begingroup\$

I was going through my Java book's exercise solutions and comparing and incorporating the author's techniques into my repertoire.

Most of the time, my approach is identical to the author's. When it is different, it is painfully obvious that his code is better and so I learn from it and rewrite it. This is the first instance where I doubt that his code is THAT much more legible and concise than mine.

Is it worth knowing both ways or should I adopt the book's solution as the "correct" answer?

1

public class PI_SUM {
    public static void main(String[] args) {
        System.out.printf("%-20s%-20s\n", "i", "m(i)");
    for (int i = 1; i <= 1000; i += 100)
        System.out.printf("%-20d%-20.4f\n", i, m(i));
    }

    public static double m(int i) {
        double pi = 0;
        double sign = 1;

    for (int k = 1; k <= i; k++) {
        pi += sign / (2 * k - 1.0);
        sign = -1 * sign;
    }

    return 4 * pi;
  }
}

2

public class PI_SUM {
public static void main(String[] args) {
    //Title and bar
    System.out.println("i\t\t\tm(i)");
    System.out.println("---------------------------------");
        
    //Table body
    for (int i = 1; i < 901; i += 100) {
        System.out.println(i + "\t\t\t" + m(i));
    }
}
//Compute summation
public static double m(int i) {
    double multiple = 0;
    //Number of sums
    double den = 1.0;
    for (int iteration = 1; iteration <= i; iteration++, den += 2) {
        if (iteration % 2 != 0)
            multiple += 1 / den;
        else
            multiple -= 1 / den;
    }
    double m = (int)((4 * multiple) * 10)) / 1000.0;
    return m;
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Our rules state that only your own code is on-topic for review. I've made a guess that sample 1 came from the book, and sample 2 is yours. Can you confirm? \$\endgroup\$ – 200_success Mar 7 '15 at 3:24
  • 2
    \$\begingroup\$ Please also include attribution for the book sample. \$\endgroup\$ – 200_success Mar 7 '15 at 3:28
  • 1
    \$\begingroup\$ It requests a review of code that doesn't belong to the poster. \$\endgroup\$ – Legato Mar 7 '15 at 4:00
  • 1
    \$\begingroup\$ Why are you dividing by 1000.0? Also why 901? The original printed out index 901 and yours doesn't. \$\endgroup\$ – JS1 Mar 7 '15 at 8:30
  • 1
    \$\begingroup\$ Putting this question on hold until the concerns I raised are addressed. \$\endgroup\$ – 200_success Mar 7 '15 at 14:41
2
\$\begingroup\$

The first piece of code is just a straight up implementation of the well-known leibniz formula. Anyone familiar with it will recognize it right away (and even find the mistake of k - 1.0 instead of k + 1.0 which I'm assuming is a typo). Apart from wrong indentation, there's nothing wrong with it.

With your code, there are a couple of issues:

  • Naming: What is a den? Or an m? And having iteration and i isn't optimal (because it's closely linked in many minds), I would go with k as the original author.
  • Unnecessary Variables: m isn't needed, just return directly.
  • the if statement to decide between - and + introduces duplication (multiple ?= 1 / den;) and unnecessary complexity. The original approach of saving the sign in a variable seems better (and shouldn't perform worse either).
  • transforming 2 * k into an addition via an additional variable might increase performance (or it might not, you should profile that), but it definitely hurts readability. And the way you are doing it is quite unclear without comments.
  • I would increase additional loop variables at the end of the loop, not inside the opening loop statement, because there they are easily overlooked.
  • as JS1 remarked in the comments, it is quite unclear why you are multiplying by 10 and then dividing by 1000. I would at the very least add a comment.
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ @mjolka oh, thanks, your right, I overlooked that. \$\endgroup\$ – tim Mar 7 '15 at 9:31

Not the answer you're looking for? Browse other questions tagged or ask your own question.