This week's book giveaway is in the Security forum.
We're giving away four copies of Securing DevOps and have Julien Vehent on-line!
See this thread for details.
Win a copy of Securing DevOps this week in the Security forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic

A simple program to simulate a die to get a desired number in row. How can I better my code?  RSS feed

 
Ranch Hand
Posts: 58
IntelliJ IDE Java Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi

I wrote a program to simulate a die. The program counts how many times the die needs to be rolled to obtain 3 consecutive sixes. However it's pretty long and contains nested loops and many variables. Is there any simpler way to achieve my objective? How might I make this code more simple so that it's easy to understand (in it's current form it's difficult to follow the flow of commands    )


Here's my code:

 
Sheriff
Posts: 4635
129
Chrome Eclipse IDE Java Postgres Database VI Editor
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
One way you might think about getting rid of the inner loop is creating a variable something like "the number of times I have already seen a six."

There are many other minor improvements that you do to your program:

* Get the code out of main(), see MainIsAPain (that's a link).

* Only declare variables right where you need them.

* The Random object should be declared only once.

* Remove comments that just say what the code says (most of your comments)

* Use more descriptive variable names (ns -> numberOfSixes)
 
charlie swift
Ranch Hand
Posts: 58
IntelliJ IDE Java Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks for the brilliant advice

I took your advice and implemented it and here's the new code. Is it better now? Any other suggestion that I implement to improve the code?



Knute Snortum wrote:One way you might think about getting rid of the inner loop is creating a variable something like "the number of times I have already seen a six."

There are many other minor improvements that you do to your program:

* Get the code out of main(), see MainIsAPain (that's a link).

* Only declare variables right where you need them.

* The Random object should be declared only once.

* Remove comments that just say what the code says (most of your comments)

* Use more descriptive variable names (ns -> numberOfSixes)

 
Bartender
Posts: 3871
47
Eclipse IDE Firefox Browser Java MySQL Database VI Editor Windows
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
On line 26 you are testing numberOfSixes!=3, you've already done that test on line 23. If you clean this up on line 26 then line  28 should become a simple 'else'.

The use of your boolean 'NotThreeSixes' is redundant, you can just test numberOfSixes!=3.
 
Master Rancher
Posts: 2410
80
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Another way, that saves you from all the bookkeeping and brings you into the realm of java 8 streams...

Suppose you have the integer sequence 1, 2, 3, .... Now, keep only those integers for which the roll of the die equals the number-to-be-rolled (call that number S), and from this reduced sequence take the first N numbers, where N is the number of times we must roll S. The last number of that reduced sequence is the required number of rolls. It should just take a few lines.
 
charlie swift
Ranch Hand
Posts: 58
IntelliJ IDE Java Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks I'll look up java streams; it sure seems like an easy way of programming the same thing and like you say "saves ... all the bookkeeping" (great phrase by the way)

Piet Souris wrote:Another way, that saves you from all the bookkeeping and brings you into the realm of java 8 streams...

Suppose you have the integer sequence 1, 2, 3, .... Now, keep only those integers for which the roll of the die equals the number-to-be-rolled (call that number S), and from this reduced sequence take the first N numbers, where N is the number of times we must roll S. The last number of that reduced sequence is the required number of rolls. It should just take a few lines.

 
Carey Brown
Bartender
Posts: 3871
47
Eclipse IDE Firefox Browser Java MySQL Database VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Piet, your java-8 solution escapes me. You realize that the criteria is not just 3 rolls of 6, but 3 consecutive rolls of six. This where I'm stuck on a java-8 solution.
 
Piet Souris
Master Rancher
Posts: 2410
80
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Aaaaggghhh, completely missed 'consecutive'..... as you rightly concluded. I apologize, I can be such an oelewapper from time to time.

That spoils it all, so stick to a simple for-loop.

Stream solutions are still possible, but too ugly to mention. Nevertheless, the shortest repair I could think of is

 
Saloon Keeper
Posts: 1754
44
Eclipse IDE Google Web Toolkit Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I tried this:

 
salvin francis
Saloon Keeper
Posts: 1754
44
Eclipse IDE Google Web Toolkit Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I think Java9's IntStream.takeWhile method would be more suitable for this task.
 
Sheriff
Posts: 11707
190
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Here's my take in Java 8:

Try it out here: https://repl.it/@jlacar/CountConsecutiveDieRolls

Edit: Refactored
 
charlie swift
Ranch Hand
Posts: 58
IntelliJ IDE Java Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks so much for the advice. After getting rid of all those redundant check/variables, the code got much easier to read.

Carey Brown wrote:On line 26 you are testing numberOfSixes!=3, you've already done that test on line 23. If you clean this up on line 26 then line  28 should become a simple 'else'.

The use of your boolean 'NotThreeSixes' is redundant, you can just test numberOfSixes!=3.

 
charlie swift
Ranch Hand
Posts: 58
IntelliJ IDE Java Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
 
charlie swift
Ranch Hand
Posts: 58
IntelliJ IDE Java Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
why can't I edit my replies/posts?
 
Sheriff
Posts: 57897
178
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Because in the past we had frequent problems with people “correcting” mistakes in old posts, so the replies looked like nonsense. Shall I correct the code tags for you?
Don't use <> for code tags: it's [], but the button is quicker. The HTML <code> tag changes the font and I have only seen it used in javadoc comments.
 
charlie swift
Ranch Hand
Posts: 58
IntelliJ IDE Java Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator


Campbell Ritchie wrote:Because in the past we had frequent problems with people “correcting” mistakes in old posts, so the replies looked like nonsense. Shall I correct the code tags for you?
Don't use <> for code tags: it's [], but the button is quicker. The HTML <code> tag changes the font and I have only seen it used in javadoc comments.



Thanks for clarifying. The reason for for not having an edit button makes sense. Yes, please correct the code tag. And I usually use the buttons for adding tags but this time i was using 'quick reply' box which has not buttons. But I'll keep in mind to use square brackets [] in the future.
 
Campbell Ritchie
Sheriff
Posts: 57897
178
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Done. Thank you for being understanding about editing.

Line 2 should have been written as two lines. Yes, I think that will work. Since you are using [pseudo]random numbers, you cannot predict how long it will take to get 666, but it should average out at 1 in 216. I presume you will therefore run the loop repeatedly and calculate an average.
 
Consider Paul's rocket mass heater.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!