Game to shuffle and guess a word I'm new to C++ and while learning, I wrote a simple program that asks for a word, shuffle sthe characters and then asks another person to find the original word, given the shuffled letters.

I would like to know what is right and wrong in this code, what and where can I improve.

#include &lt;iostream&gt;

void shuffle(std::string&amp; word);

int main(int argc, char *argv[])
{
    std::string word, shuffled, guess;
    int count(0);

    std::cout &lt;&lt; "Word to guess: ";
    std::cin &gt;&gt; word;

    shuffled = word;
    shuffle(shuffled);

    do {
        std::cout &lt;&lt; "Guess(" &lt;&lt; shuffled &lt;&lt; "): ";
        std::cin &gt;&gt; guess;
        count++;
        if (guess != word) {
            std::cout &lt;&lt; "No." &lt;&lt; std::endl;
        }
    } while (guess != word);

    std::cout &lt;&lt; "Congratulation!" &lt;&lt; std::endl;
    std::cout &lt;&lt; "Word found in " &lt;&lt; count
              &lt;&lt; " guess" &lt;&lt; (count&gt;1?"es":"")
              &lt;&lt; "." &lt;&lt; std::endl;

    return 0;
}

void shuffle(std::string&amp; word)
{
    srand(time(nullptr));
    for (unsigned int i(0); i &lt; word.length(); ++i) {
        std::swap(word[i], word[rand() % word.length()]);
    }
}
The most obvious point is probably that the standard library already includes std::shuffle, so there's little point in creating your own.


If you do decide to write your own shuffle, you probably want to look up the Fisher-Yates Shuffle algorithm. What you're doing right now can introduce biases (actually, it introduces a couple of different biases in entirely different ways).

If you're going to generate random numbers in C++, you're generally better off using the "new" generators found in &lt;random&gt; instead of using srand/rand from &lt;stdlib.h&gt;/&lt;cstdlib&gt;.


Probably also want to use the standard distribution classes instead of rolling your own with the modulus operator (that's one of your current sources of bias1).

Even if you do insist on writing your own and using srand/rand, you must call srand only once, probably when your program starts, not separately each time you're going to use some random numbers.
I advise against using std::endl. Most of the time (including this case) printing a new-line ('\n') will do what you want, and frequently be substantially faster. In addition to writing a new-line, std::endl flushes the stream, which is almost never necessary or desirable.
Instead of:

std::cout &lt;&lt; "Word found in " &lt;&lt; count
          &lt;&lt; " guess" &lt;&lt; (count&gt;1?"es":"")


I'd rather use:

std::cout &lt;&lt; "Word found in " &lt;&lt; count
      &lt;&lt; (count &gt; 1 ? "guesses" : "guess";


At least to me, this makes it somewhat easier to follow what's going on (and a trade between easier to read and saving half a dozen bytes of string constant seems pretty easy to me, at least most of the time).
Finally, Add the appropriate includes for the features you use.


std::string => #include &lt;string&gt;
std::srand() => #include &lt;cstdlib&gt;
std::time() => #include &lt;ctime&gt;
srand() => #include &lt;stdlib.h&gt;      // Note the difference between C++ and C version.
time() => #include &lt;time.h&gt;






1. consider a generator that produced 0, 1, or 2 and you used %2 to get only 0 or 1. You'd get 1 only when the generator produced 1, but you'd get 0 when it produced either 0 or 2, so assuming the generator produced 0, 1, and 2 equally frequently, your output would be 0 about twice as often as it was 1. With larger ranges, the bias tends to be less extreme (and less visible) but the same basic mechanism causes the same basic problem.