Win a copy of Programmer's Guide to Java SE 8 Oracle Certified Associate (OCA) this week in the OCAJP forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

need improvementon this code

 
Taiwo Sokunbi
Greenhorn
Posts: 9
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Android IntelliJ IDE Java
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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: 12180
34
Chrome Java Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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: 12180
34
Chrome Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Pie
Posts: 49733
69
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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: 42968
73
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
What kinds of improvements are you looking for?
 
Raymond Gillespie
Ranch Hand
Posts: 135
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Android IntelliJ IDE Java
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Pie
Posts: 10492
64
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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: 12180
34
Chrome Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Pie
Posts: 1801
28
Chrome Firefox Browser jQuery Linux MySQL Database Netbeans IDE
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Pie
Posts: 1801
28
Chrome Firefox Browser jQuery Linux MySQL Database Netbeans IDE
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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: 21298
32
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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: 12180
34
Chrome Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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: 3090
14
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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: 12180
34
Chrome Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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: 3090
14
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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: 3090
14
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic