• 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 Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Junilu Lacar
Sheriffs:
  • Rob Spoor
  • Liutauras Vilda
  • Tim Cooke
Saloon Keepers:
  • Tim Moores
  • Piet Souris
  • Tim Holloway
  • Jj Roberts
  • Stephan van Hulst
Bartenders:
  • Himai Minh
  • Carey Brown
  • Frits Walraven

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

 
Ranch Hand
Posts: 58
IntelliJ IDE Oracle Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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: 7111
184
Eclipse IDE Postgres Database VI Editor Chrome Java Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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 Oracle Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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: 8327
71
Eclipse IDE Firefox Browser MySQL Database VI Editor Java Windows
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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.
 
Saloon Keeper
Posts: 4560
182
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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 Oracle Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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: 8327
71
Eclipse IDE Firefox Browser MySQL Database VI Editor Java Windows
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
Saloon Keeper
Posts: 4560
182
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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: 2873
150
Google Web Toolkit Eclipse IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I tried this:

 
salvin francis
Bartender
Posts: 2873
150
Google Web Toolkit Eclipse IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I think Java9's IntStream.takeWhile method would be more suitable for this task.
 
Marshal
Posts: 16456
272
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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 Oracle Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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 Oracle Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
 
charlie swift
Ranch Hand
Posts: 58
IntelliJ IDE Oracle Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
why can't I edit my replies/posts?
 
Marshal
Posts: 73334
332
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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 Oracle Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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: 73334
332
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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.
 
With a little knowledge, a cast iron skillet is non-stick and lasts a lifetime.
reply
    Bookmark Topic Watch Topic
  • New Topic