19
\$\begingroup\$

I'm wondering if my code is just a tad too long to post here, but if the only consequence seems to be getting high-level reviews, then that's perfectly fine.

This is my implementation of Hangman in C++, split up into a handful of classes. I'm familiar with the syntax of curly brace languages but lack experience with the quirks and specialties of C++, which is why I'm posting my code here asking if it follows the common best practices.

Specifically, here's what I'm most interested in knowing:

  • Am I using .h and .cpp files in the right way? I'm declaring methods in the former and defining them in the latter, and I keep them in subfolders called "header" and "src" respectively.
  • Is there a widely accepted naming convention for variables and functions? Right now I'm using_underscores for functions (because I plan on doing stuff with Allegro and feel it would make sense to follow its naming scheme), and camelCasing variables because they're easier to distinguish from functions that way.
  • You may or may not notice, but I pretty much ported this code over from a Java version I made earlier. Classes like WordChooser and CharReader may have made more sense there, but in C++ they seem rather too small to deserve being classes of their own. Would they be better off as functions instead, and if so, where should they go? In main.cpp?

main.cpp

#include <GameManager.h>

int main()
{
    GameManager gm;
    gm.start_game();

    return 0;
}

GameManager.h

#pragma once

#include "Hangman.h"
#include "CharReader.h"

class GameManager {

public:
    void start_game();

private:
    Hangman hangman;
    CharReader reader;
    char guess;

    void play_game();
    void handle_guess();
    void handle_win();
    void handle_lose();
    void read_input_into_guess();
    void print_game_state_info();
    bool handle_replay();

};

GameManager.cpp

#include <iostream>
#include "Hangman.h"
#include "GameManager.h"

void GameManager::start_game()
{
    bool replay;
    do
    {
        play_game();
        replay = handle_replay();
    }
    while (replay);

    std::cout << "Alright, bye then!" << std::endl;
}

void GameManager::play_game()
{
    hangman.init_game();

    while (true)
    {
        print_game_state_info();

        read_input_into_guess();
        hangman.make_guess(guess);
        handle_guess();

        if (hangman.is_game_won())
        {
            handle_win();
            break;
        }
        if (hangman.is_game_lost())
        {
            handle_lose();
            break;
        }
    }
}

void GameManager::handle_guess()
{
    if (hangman.is_guess_correct() && hangman.is_game_won())
    {
        return;
    }
    else if (hangman.is_guess_correct())
    {
        std::cout << "correct!" << std::endl;
    }
    else
    {
        int numTries = hangman.get_number_of_tries();
        std::cout << "wrong! (" << numTries << " tr" << ((numTries == 1) ? "y" : "ies") << " left)" << std::endl;
    }
}

void GameManager::handle_win()
{
    std::cout << "You win!" << std::endl;
    print_game_state_info();
}

void GameManager::handle_lose()
{
    std::cout << "You lose!" << std::endl;
    std::cout << "The word was " << hangman.get_word() << "." << std::endl;
}

void GameManager::read_input_into_guess()
{
    std::cout << "Guess a letter: ";
    guess = reader.get_input_char();
}

void GameManager::print_game_state_info()
{
    std::cout << hangman.get_word_state_as_string();
    if (hangman.right_guesses_exist())
    {
        std::cout << "    correct: " << hangman.get_right_guesses_as_string();
    }
    if (hangman.wrong_guesses_exist())
    {
        std::cout << "    wrong: " << hangman.get_wrong_guesses_as_string();
    }
    std::cout << std::endl;
}

bool GameManager::handle_replay()
{
    char promptAnswer;
    bool replay;
    std::cout << "Play again? (Y/n)" << std::endl;
    try
    {
        promptAnswer = (char) toupper(reader.get_input_char());
        replay = (promptAnswer != 'N');
    }
    catch (int e)
    {
        replay = true;
    }

    if (replay)
    {
        std::cout << std::endl << std::endl << std::endl;
    }

    return replay;
}

Hangman.h

#pragma once

#include <string>
#include <set>
#include "WordChooser.h"

class Hangman {

public:
    Hangman();
    void init_game();
    void make_guess(char c);
    bool is_guess_correct();
    std::string get_word();
    std::string get_word_state_as_string();
    std::string get_right_guesses_as_string();
    std::string get_wrong_guesses_as_string();
    bool right_guesses_exist();
    bool wrong_guesses_exist();
    int get_number_of_tries();
    bool is_game_won();
    bool is_game_lost();

private:
    const int MAX_GUESSES = 10;
    const char PLACEHOLDER = '_';
    WordChooser chooser;
    std::string word;
    char curGuess;
    std::set<char> rightGuesses;
    std::set<char> wrongGuesses;

};

Hangman.cpp

#include "Hangman.h"
#include <algorithm>

Hangman::Hangman()
{
    init_game();
    chooser.set_file("words.txt"); // one word per line
}

void Hangman::init_game()
{
    rightGuesses.clear();
    wrongGuesses.clear();

    word = chooser.get_random_word();
    for (unsigned int i = 0; i < word.length(); i++)
    {
        word[i] = (char) toupper(word[i]);
    }
}

void Hangman::make_guess(char c)
{
    curGuess = (char) toupper(c);

    if (is_guess_correct())
    {
        rightGuesses.insert(curGuess);
    }
    else
    {
        wrongGuesses.insert(curGuess);
    }
}

bool Hangman::is_guess_correct()
{
    return (word.find(curGuess) != std::string::npos);
}

std::string Hangman::get_word()
{
    return word;
}

std::string Hangman::get_word_state_as_string()
{
    std::string output = "";
    char c;

    for (unsigned int i = 0; i < word.length(); i++)
    {
        c = word[i];
        if (std::find(rightGuesses.begin(), rightGuesses.end(), c) != rightGuesses.end()
                || !isalpha(c))
        {
            output += c;
        }
        else
        {
            output += PLACEHOLDER;
        }
    }
    return output;
}

std::string Hangman::get_right_guesses_as_string()
{
    std::string output = "";
    for (auto &&guess : rightGuesses)
    {
        output += guess;
    }
    return output;
}

std::string Hangman::get_wrong_guesses_as_string()
{
    std::string output = "";
    for (auto &&guess : wrongGuesses)
    {
        output += guess;
    }
    return output;
}

bool Hangman::right_guesses_exist()
{
    return !rightGuesses.empty();
}

bool Hangman::wrong_guesses_exist()
{
    return !wrongGuesses.empty();
}

int Hangman::get_number_of_tries()
{
    return MAX_GUESSES - wrongGuesses.size();
}

bool Hangman::is_game_won()
{
    char c;
    for (unsigned int i = 0; i < word.length(); i++)
    {
        c = (char) toupper(word.at(i));
        if (std::find(rightGuesses.begin(), rightGuesses.end(), c) == rightGuesses.end()
                && isalpha(c))
        {
            return false;
        }
    }
    return true;
}

bool Hangman::is_game_lost()
{
    return get_number_of_tries() == 0;
}

WordChooser.h

#pragma once

#include <string>
#include <fstream>

class WordChooser
{

public:
    ~WordChooser();
    void set_file(std::string filename);
    std::string get_random_word();

private:
    std::ifstream file;
};

WordChooser.cpp

#include "WordChooser.h"

#include <stdlib.h>
#include <time.h>

WordChooser::~WordChooser()
{
    file.close();
}


void WordChooser::set_file(std::string filename)
{
    file.open(filename.c_str());
}


std::string WordChooser::get_random_word()
{
    std::string output;
    std::string line;
    int n = 0;

    srand (time(NULL));

    while (!file.eof())
    {
        file >> line;
        n++;
        if (rand() % n == 0)
        {
            output = line;
        }
    }
    file.seekg(0);
    return output;
}

CharReader.h

#pragma once

class CharReader {

public:
    char get_input_char();

};

CharReader.cpp

#include "CharReader.h"

#include <iostream>


char CharReader::get_input_char()
{
    std::string input;
    std::cin >> input;

    return (char) toupper(input.at(0));
}

CMakeLists.txt (why not?)

cmake_minimum_required(VERSION 2.8.4)
project(Hangman)

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")

include_directories(header src)
set(SOURCE_FILES src/main.cpp src/Hangman.cpp src/GameManager.cpp src/CharReader.cpp src/WordChooser.cpp)
add_executable(Hangman ${SOURCE_FILES})

And lastly, here's some sample output:

____
Guess a letter: e
correct!
_E__    correct: E
Guess a letter: i
wrong! (9 tries left)
_E__    correct: E    wrong: I
Guess a letter: n
wrong! (8 tries left)
_E__    correct: E    wrong: IN
Guess a letter: s
correct!
_ES_    correct: ES    wrong: IN
Guess a letter: t
You win!
TEST    correct: EST    wrong: IN
Play again? (Y/n)
n
Alright, bye then!
\$\endgroup\$
4
\$\begingroup\$

Excellent code.

I would restructure the code to obviate the need for private member functions in GameManager.

Create a global function, start_game. Use the global function from main and leave the details of how start_game is implemented as purely encapsulated detail.

GameManager.h

#pragma once
extern void start_game();

main.cpp

#include <GameManager.h>

int main()
{
    start_game();
    return 0;
}

Then, in GameManager.cc, you can define GameManager as a class or even a struct. You add member functions that capture distinct functionality and don't have to worry about whether a function needs to be private or public

The implementation of start_game() can be:

void start_game()
{
    GameManager gm;
    gm.start_game();
}

With this approach, main.cpp doesn't need to have any knowledge of GameManager as a class.

If there is a possibility of conflicting global names, you can put start_game in a unique namespace

GameManager.h

#pragma once

namespace GameManagerNS
{
   extern void start_game();
}

and change the implementation to:

void GameManagerNS::start_game()
{
    GameManager gm;
    gm.start_game();
}
\$\endgroup\$
  • \$\begingroup\$ Thanks for the input - it was certainly useful, but I'll wait and see what others have to add. \$\endgroup\$ – vvye Jan 26 '15 at 7:35
4
+50
\$\begingroup\$

There are several style guides (e.g. google has one), it is up to you pick one or do your own but there is no universal style guide. Important is just that you are consistent in your code so that you don't use different styles in different files, that is confusing (irritating) I find your code looks fine from the style point of view.

Normally one would avoid (if possible) to include the headers like Hangman.h and CharReader.h in your header GameManager. Instead you declare them as pointers in the header and remove the headers.

class Hangman;
class CharReader;

class GameManager {

public:
  void start_game();

private:

  Hangman* hangman;
  CharReader* reader;

although the preferred way to do this is to use smart pointers:

std::unique_ptr<Hangman> hangman;
std::uniqe_ptr<CharReader> reader;

then in your GameManager.cpp you include the headers and instantiate the classes in the constructor.

GameManager()  : hangman(new Hangman), reader(new CharReader) )
{
}

Exceptions in C++ are normally not used to steer program flow as you do in handle_replay(), I think it would be better to have a simple do {} while loop to handle "replays"

When you create a project like this it is good to put it in its own namespace to avoid problems with functions overloading/collision when you don't expect it.

E.g.

namespace HangManGame
{
  class GameManager
...

other than that your code is easy to read and has good abstraction level IMHO.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.