C++ console based - "Word Guessing" game

Question

I have made a console game in C++ which is called "Word Guessing", and according to the compiler, it is using about 542 KB of memory.

I am mainly wondering if it's possible to decrease the memory usage some more, and if possible, how it can be improved.

#include <iostream>
#include <fstream>
#include <string>
#include <vector>
#include <algorithm>

using namespace std;

// Function Declaration
string getRandWord();
void checkLetter(char&, string&, int&, string&);

// main function
int main() {

    // str Variables
    string RandomWord = getRandWord();
    string hiddenWord = "";

    // int Variables
    unsigned long int _length_ = RandomWord.length();
    int chances = int(_length_)+1;
    char newLetter;

    // fill varialbe with characters
    hiddenWord.append((_length_),'_');

    cout << "The Word is "<< _length_ <<" letters long." << endl;

    // game loop
    while(chances > 0){
        // request (char) input
        cin >> newLetter;
        checkLetter(newLetter, RandomWord, chances, hiddenWord);

        // check if player won
        if(hiddenWord == RandomWord){
            cout << "Congratulations! You won the game." << endl;
            break;
        }
        // decrease try chances
        chances--;
    }

    return 0;
}


// Functions

void checkLetter(char &newLetter, string &randstr, int &strlen, string &hiddenWord){
    // initiate iterator it
    string::iterator it;

    // iterate trough string
    for(auto it = randstr.begin(); it != randstr.end();++it){
        // validate letter mach
        if(*it == newLetter){
            // replace letter
            hiddenWord.at(it - randstr.begin()) = newLetter;
        }
    }
    cout << hiddenWord << endl;
}

string getRandWord(){
    string filePath = "/Users/nedimkanat/XCODE/testcpp/testcpp/";

    // constants
    enum sizes {
        COUNTER = 0,
        ARRAY_SIZE = 5
    };

    // set seed
    srand((unsigned)time(NULL));

    // get random int between 0 and 5
    int randint = rand() % ARRAY_SIZE;

    // str to store each line from file
    string str;

    // array to store 5 (random) words
    vector<string> arr;

    // initialize file object & open file
    ifstream file(filePath+"words.txt");
    int counter = COUNTER;

    // loop trough file
    if (file.is_open()){
        while (getline(file,str) && counter < ARRAY_SIZE){
            arr.push_back(str);
            counter++;
        }
        file.close();
    } else {
        cout << "File is not open" << endl;
    }

    // send away random word
    if(arr.empty()){
        cout << "CANCER" << endl;
    }
    return arr.at(randint);
}

Answer 1

Well, there is much to improve with your code, so let's begin at the beginning:

1. You should avoid using namespace std; It's a plague and just biding its time to bite you later.
   Why is “using namespace std;” considered bad practice?

2. You might want to rework your logic regarding the tries. It's somewhat cruel to let the user go on guessing after he already cannot win anymore. And it's somewhat pointless if he can ask for every single character.

3. You should const-qualify what you can. Doing so makes logic-errors compile-errors, which can be caught by the compiler.

4. Use auto where you can. Remember that repetition is error-prone and everything you don't write nobody ever has to understand and check. Also known as the "Don't repeat yourself" (DRY) principle.

5. In that vein, I would also reorder your functions so you don't have to provide forward-declarations.

6. Consider everything but main() for static linkage, as it is never used outside the TU.

7. I'm sorry, but at best your comments are a blatant violation of DRY. Never repeat the code in the comments, write about why the code does something surprising or at least not quite straight-forward.

8. Consider consistent casing. Most of your variables begin with lower-case, why not all? Also, consider consistently having one space around binary operators.

9. There's nothing quite as useless as explicitly requesting something which will happen implicitly anyway. Aside from immediately afterwards undoing it.

   std::string hiddenWord = "";
   unsigned long int _length_ = randomWord.length();
   hiddenWord.append((_length_),'_');
   // Has the same effect as
   std::string(randomWord.length(), '_');
   

10. Avoid beginning and/or ending names with an underscore. Even if you know the rules, it's error-prone and ugly.

11. Casts, function-style or not, are a last resort. And if you must use them, consider using the new-style casts as they are more prominent and explicitly name your intent.
    Anyway, just use the right type.

12. Don't use std::endl unless you really need a manual flush. First, it's way uglier than a simple newline '\n', second, it kills performance as flushing is costly, and third, it's superfluous.
    The standard input- and output-streams std::cin and std::cout are coupled so using the latter flushes the former if used with an interactive console.

13. If you want to do something a specific number of times, use a for-loop, that's what it's for.

14. Always test whether input succeeded. A pipe for example could be closed prematurely, or a file run out.
    Also, you might want to re-try until you get something.

15. I wonder why checkLetter doesn't return whether now everything is revealed.

16. return 0; is implicit for main() in C++ and C99+.

17. rand and srand is quite a bad random-number-generator. Still, if you insist on using it, consider initializing it directly in main() to make sure you never inadvertently initialize it twice. And especially to make sure you don't repeatedly re-initialize it before using it again.
    Take a look at <random> for better RNGs.

18. There's absolutely no reason to limit yourself to the first five words in the input-file. Or to fail if the file contains less words.

19. All error output should be send to the error stream, not standard output. That's what it's there for.

20. Consider accepting the input-filename as an argument, instead of hardcoding it. You might even give it a default.

21. Extract the task of reading the opening and reading the file. std::istream_iterator might come in handy for it.

22. Why does checkLetter() accept everything by modifiable reference? hiddenWord should be, sure, but randstr (which should be named randomWord for consistency) should be a constant reference, newLetter a value and strlen is superfluous.

23. That function also has two variables named it, the first one should be summarily removed.

Modified code:

#include <iostream>
#include <fstream>
#include <string>
#include <vector>
#include <algorithm>
#include <iterator>
#include <string_view>

static bool checkLetter(
    char newLetter,
    std::string_view randomWord,
    std::string &hiddenWord
) {
    for(auto it = randomWord.begin(); it != randomWord.end(); ++it)
        if(*it == newLetter)
            hiddenWord.at(it - randomWord.begin()) = newLetter;
    std::cout << hiddenWord << '\n';
    return randomWord == hiddenWord;
}

static auto readFile(const char* filename) {
    std::ifstream file(filename);
    using isi = std::istream_iterator<std::string>;
    std::vector<std::string> arr{isi(file), isi()};
    return arr;
}

static void removeBadEntries(std::vector<std::string>& arr) {
    std::sort(arr.begin(), arr.end());
    arr.resize(std::remove_if(arr.begin(), arr.end(),
        [last = std::string_view()](auto& x) mutable{
            for(auto y : x)
                if(!isgraph((unsigned char)y))
                    return true;
            if(last == x)
                return true;
            last = x;
            return false;
        }) - arr.begin());
}

static auto getRandWord(const char* filename) {
    auto arr = readFile(filename);
    removeBadEntries(arr);
    std::cout << "Choosing from " << arr.size() << " unique valid entries.\n";

    return arr.empty() ? std::string("CANCER") : std::move(arr[rand() % arr.size()]);
}

int main(int argc, char* argv[]) {
    std::setlocale(LC_ALL, "");
    srand((unsigned)time(NULL));

    const auto randomWord = getRandWord(argc > 1
        ? argv[1]
        : "/Users/nedimkanat/XCODE/testcpp/testcpp/words.txt");
    std::string hiddenWord(randomWord.length(), '_');

    std::cout << "The Word is " << randomWord.length() << " letters long.\n";

    for(auto chances = randomWord.length() + 1; chances > 0; --chances){
        char c;
        do {
            if(!(std::cin >> c)) {
                std::cerr << "Error reading a character from stdin.\n";
                return EXIT_FAILURE;
            }
        } while(!std::isgraph((unsigned char)c));

        if(checkLetter(c, randomWord, hiddenWord)) {
            std::cout << "Congratulations! You won the game.\n";
            break;
        }
    }
}