This week's book giveaway is in the Artificial Intelligence forum.
We're giving away four copies of Pragmatic AI and have Noah Gift on-line!
See this thread for details.
Win a copy of Pragmatic AI this week in the Artificial Intelligence forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Jeanne Boyarsky
  • Liutauras Vilda
  • Campbell Ritchie
  • Tim Cooke
  • Bear Bibeault
Sheriffs:
  • Paul Clapham
  • Junilu Lacar
  • Knute Snortum
Saloon Keepers:
  • Ron McLeod
  • Ganesh Patekar
  • Tim Moores
  • Pete Letkeman
  • Stephan van Hulst
Bartenders:
  • Carey Brown
  • Tim Holloway
  • Joe Ess

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: 5037
138
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: 4545
50
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: 2758
93
  • 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: 4545
50
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: 2758
93
  • 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

 
Bartender
Posts: 1847
46
Eclipse IDE Google Web Toolkit Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I tried this:

 
salvin francis
Bartender
Posts: 1847
46
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: 12199
199
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?
 
Marshal
Posts: 59786
188
  • 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
Marshal
Posts: 59786
188
  • 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.
 
It is sorta covered in the JavaRanch Style Guide.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!