• 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
  • Ron McLeod
  • Paul Clapham
  • Jeanne Boyarsky
  • Liutauras Vilda
Sheriffs:
  • Rob Spoor
  • Bear Bibeault
  • Tim Cooke
Saloon Keepers:
  • Tim Moores
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Piet Souris
Bartenders:
  • Frits Walraven
  • Himai Minh

Is this "guess the word program" (hangman) correct ?

 
Greenhorn
Posts: 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I've written this guess the word game program. It seems to work fine, but I feel there's something wrong about it. Could you please review the code and see if there's something wrong with or are there any
optimizations I can make to it. Thank you in advance.

 
Saloon Keeper
Posts: 8243
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
Math.random() has been replaced by the Random class which has a cleaner interface.


Don't put everything in main().
 
Rancher
Posts: 4316
38
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Continually creating a new String from guessedLetters looks wrong.  Create it only as needed.
 
Carey Brown
Saloon Keeper
Posts: 8243
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
Some minor suggestions.
Make things that don't change into constants, e.g. INPUT, RAND, WORDS.
You could use StringBuilder instead of a char[]. Builder is very similar to a String but is muttable so you can add and delete directly.
Make the loop that checks to see if you want to play again separate from the actual play loop.
Make getRandomWord() method which allows future enhancement to get words from file for example.

 
Marshal
Posts: 73021
330
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
What's the point of indexOf(...) > −1 when you can write contains(...)? The inequality might seem clever, but it is potentially more error‑prone than a simple method call.
 
Sheriff
Posts: 16282
271
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
This would be a nice little exercise in refactoring and adding unit tests. I'll post something in the Refactoring forum if you'd like to follow along.

Here's the refactoring thread: https://coderanch.com/t/743010/engineering/Refactoring-main -- everyone is welcome to follow along and contribute ideas.
 
Junilu Lacar
Sheriff
Posts: 16282
271
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
One thing I've discovered while putting your program under test is that it accepts '*' as input and incorrectly reports it as "already in the word."

Here's the test that confirmed that behavior:

Here's the result of running the test (failure)

> Task :test FAILED
...
expected: 0
but was : 2
 
Junilu Lacar
Sheriff
Posts: 16282
271
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

Carey Brown wrote:Some minor suggestions.
Make things that don't change into constants, e.g. INPUT, RAND, WORDS.
You could use StringBuilder instead of a char[]. Builder is very similar to a String but is muttable so you can add and delete directly.
Make the loop that checks to see if you want to play again separate from the actual play loop.
Make getRandomWord() method which allows future enhancement to get words from file for example.


It's interesting how similar yet different some of these choices are to the ones I've made in my refactoring attempt. The extraction to getRandomWord() is one thing that I did also but for an entirely different reason. In my case, the motivation was to create a seam that I could use to make the code more testable.

Another thing I noticed is around the Scanner object being used as the means of input. In my refactoring, I didn't extract it to a constant but rather, I extracted it to another method that I could also use as a seam to bring the means of input under control of the test. This couldn't be achieved by the change that you made to it by extracting to a constant and initializing it on class loading.

I did find that I eventually wanted to have the main() method look like what you have in order to separate the "play again?" logic from the "play once" logic. See my progress in the thread in the Refactoring forum if you want more details and understand the rationale behind the changes I made while refactoring this code. My first main objective was to clarify that big noisy section of code that makes up the bulk of the logic of the "play once" part of the program.
 
lowercase baba
Posts: 12989
66
Chrome Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
the best way to tell if your program is correct is to compare it against your specifications for what it is supposed to do.  We don't have those specs, so we can't actually tell you if it is correct or not.

and there are almost always optimizations you can make, but a better questions is are there any optimizations you should make.  And the answer to that again goes back to your specs, and what they say about efficiency/speed.  it's almost always better to write clean, understandable, maintainable code that super-efficient code.
 
Junilu Lacar
Sheriff
Posts: 16282
271
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

fred rosenberger wrote:We don't have those specs, so we can't actually tell you if it is correct or not.


True but we can make some pretty good educated guesses based on how the code is written. Ironically, some of those clues come from the comments, which are normally a code smell that indicates a lack of clarity and expressiveness in the code itself.

For example, this code:

The comment and the message being displayed would lead me to guess that the intended behavior is not in line with the actual behavior. It seems that if the word has multiple instances of the same letter, *all* of the instances are expected to be revealed. However, as it was written, the code only reveals one instance of the letter for each guess even if there are multiple instances of that letter in the word. In my testing of the code, I specified "foo" as the word and guessing 'o' will only revealed the first one. I had to guess 'o' a second time to reveal the second 'o'. That behavior is verified by this failing test:

This fails both assertions:

Multiple Failures (2 failures)
org.opentest4j.AssertionFailedError:
expected: 1
but was : 3
org.opentest4j.AssertionFailedError:
expected: 2
but was : 0

This failure shows that the program as written will not display multiple instances of the same letter the first time it is guess but will in fact require the player to guess the same letter as many times as there are instances of it in the word.
 
Ranch Foreman
Posts: 484
10
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Carey Brown wrote:Math.random() has been replaced by the Random class which has a cleaner interface.
...



This is not the first or second time I have heard that Math.random() is deprecated...

I have been on the "Read the Javadocs" bandwagon this week!

I note this:

Many applications will find the method Math.random() simpler to use.


At:
https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/util/Random.html

So people here are steering us away from Math.random(), which is good.
The Javadocs, even at V16, are steering people who like 'easy' towards it for many applications.

When we look at the Javadocs for Math.random() itself, we don't see a deprecation either:
https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/lang/Math.html#random()

So as Seinfeld used to say: "Hey, What's the deal with Math.random()?"
 
Junilu Lacar
Sheriff
Posts: 16282
271
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

Jesse Silverman wrote:

Carey Brown wrote:Math.random() has been replaced by the Random class which has a cleaner interface.


So people here are steering us away from Math.random(), which is good.
The Javadocs, even at V16, are steering people who like 'easy' towards it for many applications.

When we look at the Javadocs for Math.random() itself, we don't see a deprecation either:
https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/lang/Math.html#random()

So as Seinfeld used to say: "Hey, What's the deal with Math.random()?"


I suspect the deal is that nobody has really cared enough to update that one sentence in the JavaDocs. Digging back, it was already there since Java 5 and I would bet it has been there all along from the very beginning. If you think about it, "simpler to use" is highly subjective; it's just that around here, our subjectivity leans towards java.util.Random when we talk about simplicity and, of course, we like to think we're more the norm than the exception.
 
Jesse Silverman
Ranch Foreman
Posts: 484
10
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thanks.  This week I saw open bug reports about FileStream/FilePrint javadocs issues that weren't just minor like this but just plain wrong, which caused me to wrongly believe how methods in them worked, and they had been open for 18 years!!!

So, it is bad not to read the Javadocs concerning anything you are doing, but maybe like various authorities, it is best not to trust them overly much without further checking.

Microsoft has open-sourced a TREMENDOUS amount of their developer-facing documentation.  You can easily open issues and monitor the progress on GitHub, or for others, they are now even letting you submit PULL REQUESTS with fixes you provide yourself, if approved.

Yes, this is Tom Sawyer getting people to whitewash the fence for free, maybe, but I feel like I reported 16 docs bugs or so to them in the last year and like 10 have already been fixed, the others are just waiting on big section re-writes that had been identified as necessary or are claimed to be about to vanish when they re-instrument a toolchain in the case of rendering issues, etc.

Is OpenJDK not in charge of the JavaDocs for things delivered as part of OpenJDK?  I am not sure how any of that works in 2021.
reply
    Bookmark Topic Watch Topic
  • New Topic