Win a copy of Mesos in Action this week in the Cloud/Virtualizaton forum!

need improvementon this code

Taiwo Sokunbi
Greenhorn
Posts: 9
The program is to calculate the value of PI for the first 200th terms of the series : PI= 4 - 4/3 +4/5 - 4/7 + 4/9 - 4/11 + ...

public class FindingValueOfPie
{
public static void main(String []args)
{
double numerator = 4.0;

double pie = 0.0 , pie2=0.0;

for (int denominator=1; denominator<=2001; denominator+=4)
{

pie =pie + numerator/denominator;

}

for(int denominator2=3; denominator2<=2001 ; denominator2 +=4)

pie2 -= (numerator/denominator2);

pie= pie + pie2;

System.out.printf("final pie is : %.8f", pie);

}

}

Jeff Verdegan
Bartender
Posts: 6109
6
• 1
Hi, and welcome to the Ranch!

Your code will be easier to read (which will make it easier for people to help you) if you UseCodeTags(←click) along with reasonable indenting and white space.

As for your question, for a start, you can't get 200 terms (or are you trying to get 2000) using double, since double only carries about 16 decimal digits of precision. You'll need to use BigDecimal instead.

fred rosenberger
lowercase baba
Bartender
Posts: 12124
30
• 1
Here is your code cleaned up, properly formatted, and put in code tags. See how much easier it is to read?

fred rosenberger
lowercase baba
Bartender
Posts: 12124
30
suggestions:

1) comment your code. This should be inviolate. There is no reason not to put in SOME kind of comments.

2) ALWAYS use curly braces in every for statement, even if the body is only one line.

Campbell Ritchie
Sheriff
Posts: 48954
60
Why are you using += 4 and <= 2001? You might find < easier to read than <= (< 2002, then). Also, I think you will only get 1000 terms from that, if you increase by 4 every time.

Ulf Dittmer
Rancher
Posts: 42967
73
What kinds of improvements are you looking for?

Raymond Gillespie
Ranch Hand
Posts: 135
fred rosenberger wrote:suggestions:

1) comment your code. This should be inviolate. There is no reason not to put in SOME kind of comments.

2) ALWAYS use curly braces in every for statement, even if the body is only one line.

My C++ instructor said that it is better to not use curly braces if you only have one statement for any conditional statements because they junk up the code. It seems to me that it could be better to always use them that way one gets in the habit of it. What is your reasoning?

Jeff Verdegan
Bartender
Posts: 6109
6
• 1
Raymond Gillespie wrote:
fred rosenberger wrote:suggestions:

1) comment your code. This should be inviolate. There is no reason not to put in SOME kind of comments.

2) ALWAYS use curly braces in every for statement, even if the body is only one line.

My C++ instructor said that it is better to not use curly braces if you only have one statement for any conditional statements because they junk up the code. It seems to me that it could be better to always use them that way one gets in the habit of it. What is your reasoning?

I can't answer for Fred, but I'll bet his thoughts are pretty clsoe to mine here: 1) Yes, being in the habit of using them is one good reason. It just simplifies things. You always use the braces, no need to spend a millisecond's thought on whether this situation is a brace or no-brace one. Less complexity means fewer chances for mistakes. 2) If you need to add additional statements to the block, there's no chance that you'll forget to also add the braces. 3) With the braces, it's clear to anybody reading the code later (including you--believe me, you'll forget what you were thinking pretty quickly) exactly which statement(s) are meant to be included in the block. If you don't use braces, people (including you) might wonder whether that was intentional. Making your intent clear to whomever reads your code in the future is very important.

While I'm in favor of uncluttered code, in this case, I think a couple extra braces are well worth it.

Winston Gutkowski
Bartender
Posts: 10417
63
Taiwo Sokunbi wrote:The program is to calculate the value of PI for the first 200th terms of the series : PI= 4 - 4/3 +4/5 - 4/7 + 4/9 - 4/11 + ...

Well, it's a while (several whiles in fact) since I did algebra, but I can see one factorization that might simplify things.

Jeff Verdegan wrote:As for your question, for a start, you can't get 200 terms (or are you trying to get 2000) using double, since double only carries about 16 decimal digits of precision. You'll need to use BigDecimal instead.

Actually, I'd have to disagree there. You can certainly get 200 terms, it's just that their sum won't be accurate to the full precision of a double. I reckon it should be good to 12 DP's or so though.

@Taiwo: What Jeff said is a very important thing to know about doubles (or floats); their values are NOT always exact. You might want to read this page on the subject.

And if you need to be able to guarantee the accuracy of your answer, he's absolutely right: use BigDecimal.

Winston

fred rosenberger
lowercase baba
Bartender
Posts: 12124
30
Raymond Gillespie wrote:
fred rosenberger wrote:suggestions:

1) comment your code. This should be inviolate. There is no reason not to put in SOME kind of comments.

2) ALWAYS use curly braces in every for statement, even if the body is only one line.

My C++ instructor said that it is better to not use curly braces if you only have one statement for any conditional statements because they junk up the code. It seems to me that it could be better to always use them that way one gets in the habit of it. What is your reasoning?

yup - pretty much what Jeff said. to further illustrate the point, look at your original post, where you have this:

I had to stop and really look at your code for a while. because of the poor formatting, it isn't clear if you meant for the "pie = pie + pie2" to be part of the for loop or not. Putting in the bracket make it INSTANTLY clear that it is NOT a part of the loop.

and I cannot tell you the number of hours of time I have lost when, after not using curly braces, i tried to debug my code by putting "System.out.println()" statements in my for-loops. So the above became:

putting in the print statements made things go REALLY wrong, wasting more time than I care to admit.

Jeff Verdegan
Bartender
Posts: 6109
6
Winston Gutkowski wrote:
Jeff Verdegan wrote:As for your question, for a start, you can't get 200 terms (or are you trying to get 2000) using double, since double only carries about 16 decimal digits of precision. You'll need to use BigDecimal instead.

Actually, I'd have to disagree there. You can certainly get 200 terms, it's just that their sum won't be accurate to the full precision of a double. I reckon it should be good to 12 DP's or so though.

Ah, right, of course. I was conflating "terms in the series" with "digits of precision." My bad. Sorry for any confusion.

What will happen, as at some point, adding more terms won't change the result any. I don't know what that number of terms will be, but it will be larger than doubles 16(?) digits of precision.

Taiwo Sokunbi
Greenhorn
Posts: 9
Oops, that is typo error, it's suppose to be 2000th terms. And thanks for the constructive criticism, I will try to improve on those .

J. Kevin Robbins
Bartender
Posts: 1759
19
Raymond Gillespie wrote:
My C++ instructor said that it is better to not use curly braces if you only have one statement for any conditional statements because they junk up the code. It seems to me that it could be better to always use them that way one gets in the habit of it. What is your reasoning?

Wow. I don't think I could possibly disagree more. That's terrible advice. Extra braces don't clutter up the code, they make it more readable. That's like saying comments just clutter up the code. The other reasons for always using braces have already been mentioned so I won't repeat them.

Jeff Verdegan
Bartender
Posts: 6109
6
Jk Robbins wrote:That's like saying comments just clutter up the code.

There are a lot of comments out there that are nothing more than clutter. They add negative value to the code. I can't tell you how many times I've seen this:

Cruft like this exists in a lot of production code. In some cases they're just generated, but that's not really any more forgivable. Either the developer should have configured his IDE not to generate that junk, or should have gotten rid of them after they were generated, or the IDE writers shouldn't have done it in the first place.

Comments should only exist if they add useful information or clarify something.

J. Kevin Robbins
Bartender
Posts: 1759
19
Jeff Verdegan wrote:
Jk Robbins wrote:That's like saying comments just clutter up the code.

Comments should only exist if they add useful information or clarify something.

Good point on the useless comments. I guess that was a bad analogy. The point I was trying to make is that the extra braces do provide useful information and they do clarify things.

Jeff Verdegan
Bartender
Posts: 6109
6
Jk Robbins wrote:The point I was trying to make is that the extra braces do provide useful information and they do clarify things.

Absolutely.

Paul Clapham
Sheriff
Posts: 21107
32
Jeff Verdegan wrote:As for your question, for a start, you can't get 200 terms (or are you trying to get 2000) using double, since double only carries about 16 decimal digits of precision. You'll need to use BigDecimal instead.

I don't think that BigDecimal is going to be all that helpful, since the second term is a multiple of 1/3. You have the same problem here that you do with doubles... you have to choose where to truncate the decimal representation.

Anyway I think that double versus BigDecimal is an unnecessary diversion, since it looks to me like the goal of this exercise is to have somebody write a code which does a loop which is almost simple. The accuracy thing is a red herring because the sequence in question converges to pi more slowly than any other known sequence, and the answer (even to 2000 terms) isn't going to be especially close to pi.

fred rosenberger
lowercase baba
Bartender
Posts: 12124
30
Paul Clapham wrote:the answer...isn't going to be especially close to pi.

well, not for some definitions of "especially close"

Mike Simmons
Ranch Hand
Posts: 3076
14
I'd call it not remotely close, for all the computation it requires. Yeah, it's converging, eventually. But it's such an inefficient formula, the added precision of BigDecimal is irrelevant. The added processing time and poor readability are relevant, however.

Clearly this is a teaching problem. And hopefully the instructor will give a problem later that forces students to use BigDecimal so they can appreciate how much better it is at high-precision answers. This, however, is not that problem.

Jeff Verdegan
Bartender
Posts: 6109
6
Paul Clapham wrote:
Jeff Verdegan wrote:As for your question, for a start, you can't get 200 terms (or are you trying to get 2000) using double, since double only carries about 16 decimal digits of precision. You'll need to use BigDecimal instead.

I don't think that BigDecimal is going to be all that helpful,

Yeah, that was a result of my fuzzy thinking confusing "terms of the series" with "digits of precision."

fred rosenberger
lowercase baba
Bartender
Posts: 12124
30
Mike Simmons wrote:I'd call it not remotely close, for all the computation it requires.

but in absolute terms, the answer I got when running the code was off by 0.0009989964...

for a lot of practical applications, that is close enough.

Mike Simmons
Ranch Hand
Posts: 3076
14
But it's not just that, Jeff. Using BigDecimal would be very sensible advice on many problems that look similar to this. But the formula given is extremely inefficient - evaluating 2000 terms gives barely 4 digits of accuracy here. Using double is more than up to the task. Even float is up to the task. But if the formula were more efficient, using BigDecimal would make a big difference.

Mike Simmons
Ranch Hand
Posts: 3076
14
fred rosenberger wrote:
Mike Simmons wrote:I'd call it not remotely close, for all the computation it requires.

but in absolute terms, the answer I got when running the code was off by 0.0009989964...

for a lot of practical applications, that is close enough.

Yeah, but it's also just barely more accurate than using 22/7.

But this is why I said "I'd call it..." rather than "it is...". How close is "good enough" depends a lot on the context. I'm just saying the inaccuracy of the formula is much greater than the difference between using double and BigDecimal, unless you use a lot more terms.

Jeff Verdegan
Bartender
Posts: 6109
6
Mike Simmons wrote:But it's not just that, Jeff. Using BigDecimal would be very sensible advice on many problems that look similar to this. But the formula given is extremely inefficient - evaluating 2000 terms gives barely 4 digits of accuracy

Right, I realize that. When I first suggested BD though, I wasn't thinking about the forumla at all, just at the number 2000. That's where that red herring was born. I really wish I had paid closer attention.