Hangman Game in C#

Question

I've created a simple Hangman game (not finished yet).

• What could I improve? (This isn't intended to be object-oriented but suggestions on that are also welcome.)

• What are its flaws?

Here's the code:

public partial class Game : Form
{
 private ReadFromFile readFromFile;
 private Random random;
 private string word;
 private int wrongGuesses;
 private int amountOfLettersRevealed;
 private bool wordRevealed;
 private const int MAX_WRONG_GUESSES = 9;
 public Game()
 {
 InitializeComponent();
 }
 private void Game_Load(object sender, EventArgs e)
 {
 loadWords("words");
 selectRandomWord();
 }
 private void loadWords(string fileLocation)
 {
 readFromFile = new ReadFromFile(fileLocation);
 readFromFile.read();
 }
 private void selectRandomWord()
 {
 random = new Random();
 string[] words = readFromFile.lines;
 word = words[random.Next(words.Length)];
 }
 private void letter_Click(object sender, EventArgs e)
 {
 if (wordRevealed)
 return;
 Button button = (Button)sender;
 char letter = char.Parse(button.Name.Substring(6).ToLower());
 guessLetter(letter);
 buttonClicked(button);
 }
 private void guessLetter(char letter)
 {
 if (!word.Contains(letter))
 {
 // wrong guess
 wrongGuesses++;
 }
 else
 {
 // right guess
 for (int index = 0; index < word.Length; index++)
 {
 if (letter == word[index])
 {
 revealLetter(index, letter);
 amountOfLettersRevealed++;
 }
 }
 }
 if (amountOfLettersRevealed == word.Length)
 {
 wordRevealed = true;
 gameWon();
 }
 if (wrongGuesses == MAX_WRONG_GUESSES)
 {
 gameOver();
 }
 }
 private void buttonClicked(Button button)
 {
 button.Enabled = false;
 Image image = loadImage(button.Name.Substring(6) + "_HOV");
 button.BackgroundImage = image;
 }
 private void revealLetter(int index, char letter)
 {
 }
 private void gameWon()
 {
 playSound("youwin");
 }
 private void gameOver()
 { 
 playSound("gameover");
 }
 private Image loadImage(string location)
 {
 object o = Resources.ResourceManager.GetObject(location);
 return (Image) o;
 }
 private void playSound(string sound)
 {
 SoundPlayer sp = new SoundPlayer(sound + ".wav");
 sp.Play();
 }
}

Answer 1

private void loadWords(string fileLocation)

Follow standard naming conventions: private void LoadWords(string fileLocation) - same for the rest of your methods.

In private void guessLetter(char letter) there is the following code:

if (amountOfLettersRevealed == word.Length)
{
 wordRevealed = true;
 gameWon();
}
if (wrongGuesses == MAX_WRONG_GUESSES)
{
 gameOver();
}

Presumably if amountOfLettersRevealed == word.Length then wrongGuesses != MAX_WRONG_GUESSES? In that case it seems to make more sense to use else if, rather than evaluating the second expression even if the first is true. (Why would you call gameWon() and then check if you lost too?)

Also I think that code should not be in that method at all. The method is called guessLetter. It should handle the logic of guessing the letter. Checking whether the player has won or lost is not "guessing the letter".

Also it seems strange to have the letter_Click call a buttonClicked method. Initially I thought that was the button's click event handler due to the similarity between this method's name and the default name for a click event handler for a nameless button. I would maybe call that method UpdateButtonBackground or whatever that describes what that method actually does, since it's not an event handler.

• Anonymous: I am sorry, I am used to Java (camelCase). I'll change that to PascalCase. | That's completely true, I hadn't noticed that. | I understand, the name of the method doesn't suit its behaviour. Would you recommend to create 2 extra methods called "checkIfWon" and "checkIfLost" and call them instead? | UpdateButtonBackground still doesn't describe its behaviour since I disable the button too. | Thank you so much for your feedback!
• 404: I'd be inclined to put the code to check if the player has won or lost in a single method, and possibly called by the same code block that calls guessLetter() (not called from within guessLetter() itself).
• Anonymous: What approach do you recommend? Call checkIfWonOrLost in the letter_Click method?
• 404: Yes, why not. If letter_Click calls guessLetter then I think it should also call checkIfWonOrLost. So from that location you can see that the letter is checked, and then the outcome is checked.
• Anonymous: Ok :). Thanks man! Here is the updated version: pastebin.com/yk7wJ0wn

Answer 2

In short methods like this one:

private Image loadImage(string location)
{
 object o = Resources.ResourceManager.GetObject(location);
 return (Image) o;
}

I would just do this:

private Image loadImage(string location)
{
 return (Image)Resources.ResourceManager.GetObject(location);
}

But it looks like you are only calling this method once, so there is really no need to have a separate method for this.

You should probably also implement the RevealLetter method. I also noticed that there is no logic for showing a hidden word to the user (like how many letter are in the word to guess. Is that intentional? it might be nice to have that for the user.

• Anonymous: I'll update that method. It was intented for many usages so that's why I created a method for it (even though I only use it once right now). I plan to finish "RevealLetter" as soon as I finish the GUI. The amount of letters in a word will also be shown when the GUI is finished. Thank you!