This week's book giveaway is in the Servlets forum.
We're giving away four copies of Murach's Java Servlets and JSP and have Joel Murach on-line!
See this thread for details.
The moose likes Beginning Java and the fly likes need improvementon this code Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Murach's Java Servlets and JSP this week in the Servlets forum!
JavaRanch » Java Forums » Java » Beginning Java
Bookmark "need improvementon this code" Watch "need improvementon this code" New topic
Author

need improvementon this code

Taiwo Sokunbi
Greenhorn

Joined: Feb 01, 2012
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

Joined: Jan 03, 2004
Posts: 6109
    
    6

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

Joined: Oct 02, 2003
Posts: 11153
    
  16

Here is your code cleaned up, properly formatted, and put in code tags. See how much easier it is to read?


There are only two hard things in computer science: cache invalidation, naming things, and off-by-one errors
fred rosenberger
lowercase baba
Bartender

Joined: Oct 02, 2003
Posts: 11153
    
  16

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

Joined: Oct 13, 2005
Posts: 37936
    
  22
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
Marshal

Joined: Mar 22, 2005
Posts: 41068
    
  43
What kinds of improvements are you looking for?


Ping & DNS - my free Android networking tools app
Raymond Gillespie
Ranch Hand

Joined: Oct 08, 2012
Posts: 105
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

Joined: Jan 03, 2004
Posts: 6109
    
    6

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

Joined: Mar 17, 2011
Posts: 7503
    
  18

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


Isn't it funny how there's always time and money enough to do it WRONG?
Articles by Winston can be found here
fred rosenberger
lowercase baba
Bartender

Joined: Oct 02, 2003
Posts: 11153
    
  16

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

Joined: Jan 03, 2004
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

Joined: Feb 01, 2012
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

Joined: Dec 16, 2010
Posts: 826
    
  13

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.


"The good news about computers is that they do what you tell them to do. The bad news is that they do what you tell them to do." -- Ted Nelson
Jeff Verdegan
Bartender

Joined: Jan 03, 2004
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

Joined: Dec 16, 2010
Posts: 826
    
  13

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

Joined: Jan 03, 2004
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
Bartender

Joined: Oct 14, 2005
Posts: 18541
    
    8

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

Joined: Oct 02, 2003
Posts: 11153
    
  16

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

Joined: Mar 05, 2008
Posts: 2983
    
    9
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

Joined: Jan 03, 2004
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

Joined: Oct 02, 2003
Posts: 11153
    
  16

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

Joined: Mar 05, 2008
Posts: 2983
    
    9
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

Joined: Mar 05, 2008
Posts: 2983
    
    9
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

Joined: Jan 03, 2004
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.
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: need improvementon this code
 
Similar Threads
java fraction class
Wrong output
Help with error in running program
Fractions
no clue