Win a copy of Spring in Action (5th edition) this week in the Spring 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:
  • Campbell Ritchie
  • Bear Bibeault
  • Devaka Cooray
  • Liutauras Vilda
  • Jeanne Boyarsky
Sheriffs:
  • Knute Snortum
  • Junilu Lacar
  • paul wheaton
Saloon Keepers:
  • Ganesh Patekar
  • Frits Walraven
  • Tim Moores
  • Ron McLeod
  • Carey Brown
Bartenders:
  • Stephan van Hulst
  • salvin francis
  • Tim Holloway

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: 5446
147
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)

 
Saloon Keeper
Posts: 5140
54
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: 3001
105
  • 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
Saloon Keeper
Posts: 5140
54
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: 3001
105
  • 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: 1970
57
Eclipse IDE Google Web Toolkit Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I tried this:

 
salvin francis
Bartender
Posts: 1970
57
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: 12746
210
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: 61712
193
  • 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: 61712
193
  • 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!