Rock Paper Scissor Code

Here is the code. Works great! Any advice would be great! Also, try it BEFORE reading the code. See if it is good. I've only found 1 "exploit" that can be use to beat the computer everytime and it is still in progress.

NOTE : All class are in one folder because of simplicity and other reasons

Git repo., you can download the .exe here : https://github.com/DatHA3990/rockpaperscissor.


How to play:


Choose between rock : press 1, paper : press 2, or scissor : press 3.


Then press ENTER.

Here is the code:

```
/*

Code written by Dat HA
Copyrights 2017/03/30

This code used to be in multiple folders, but do to compiler problems and many more,
I've decided to pack them all in to this one.

I used multiple - if - else if - else - because it is reliable and clean.

*/

#include // I/O library
#include // string library
#include // vector library

#define VERSION_NUM 1.21 // version number

#define ROCK 1
#define PAPER 2
#define SCISSOR 3

#define TIE 0

#define LOSS 1
#define WIN 2

#define HUMAN 1
#define CPU 2

// CALL UPDATE EACH TIME!!!!!
class CpuPlayer { // cpu player class
private:
int m_move = 0; // cpu's move

bool m_firstMove = 1; // true = this is the first move : false = we've past the first move
static const int m_cpuFirstMove = PAPER; // cpu's first move will be paper

int m_totalGames = 0; // total games

std::vector m_gameResult{ 0 }; // the game results - 0 - tie or error - 1 = human player win - 2 = cpu win
std::vector m_ennemiMove{ 0 }; // ennemi's moves are logged here
std::vector m_cpuMove{ 0 }; // cpu's moves are lagged here

void FirstMove();

void Loss(); // we previously loss
void Win(); // we previously won
void Tie(); // we previously tied

void Double(); // the opponent played the same move 2 times
void Spam(); // the opponent is spaming a move (because they are trying to crack the algo!!!!)
void Reverse(); // opponent is going in reverse order, l

I built this and played it. I managed to win a few times! Good times.

Overall, the code is straightforward and easy to understand, so props on that. Here are a few things that could use improvement.

Playability

First, I think you should print out the instructions, at least the first time the user plays. Just what you have written in your description above the code would suffice. My first instinct was to type "Rock" instead of "1". Which leads to the next point:

Error handling

There's no error checking at all in this. If the user enters something incorrect, nothing happens. No error message, no exiting the program, it just sits at the prompt and waits. I'd expect it to tell me the input was invalid and to attempt to get it again. Something like this:

int HumanPlayer::GetMove() {
    int result = ROCK;
    bool valid = false;
    do {
        std::cout >>";

        std::string string;
        std::cin >> string;

        if (string == "1")
        {
            result = ROCK;
            valid = true;
        }
        else if (string == "2")
        {
            result = PAPER;
            valid = true;
        }
        else if (string == "3")
        {
            result = SCISSOR;
            valid = true;
        }
        else 
        {
            std::cout << "Invalid input. Please enter 1, 2, or 3.\n";
        }
    } while (!valid);

    return result;
}

Bugs

Notice that I removed the call to this->GetMove() in the final else clause. I did this because it was causing 2 bugs:

• When it executes, the return value of the function is undefined

• It can cause an infinite recursion, which eventually crashes the app. In my case I hit Cntl-D (End of File) and it spit out hundreds of ">>>"s and then crashed when the stack ran into the heap.

By making it non-recursive we eliminate the possibility of overflowing the stack. We also make it easier to follow.

Use Proper Types

C++ has a decent type system. You should use it to avoid errors. Instead of having a bunch of #defines at the top of your source file, you should create some types that express your meaning. Also, you should use constants rather than macros in C++ as they are safer and include type information. I recommend something like this:

const std::string kVersionNum("1.21");

typedef enum Attack {
    kRock = 1,
    kPaper = 2,
    kScissor = 3
} Attack;

typedef enum Outcome {
    kTie = 0,
    kLoss = 1,
    kWin = 2
} Outcome;

typedef enum User {
    kHuman = 1,
    kCPU = 2
} User;

I'm using the style of prefixing constants with k. If you prefer all caps, you can use that, but I find that style a little dated and hard to read, personally. Whatever you choose, be consistent.

AI

I really like your AI! It's very clever to base your next move on either your previous moves or the human's previous moves.

Simplify

There are a few things you could simplify. Both methods of the Translate class, and the HumanPlayer::GetMove() function could be implemented with a simple lookup table. As could all the CpuPlayer moves (Loss(), Win() etc.). Here's how it would look for Translate::TieLossWin():

std::string Translate::TieLossWin(int val) {
    const std::string kResultStrings[] = {
        std::string("TIE"),
        std::string("HUMAN WIN"),
        std::string("CPU WIN")
    };
    if (val < 3)
    {
        return kResultStrings [ val ];
    }
    else
    {
        return "SYSTEM ERROR";
    }
}

For HumanPlayer::GetMove() you could translate their string into an int and then use it as a look-up in a table similar to the above example. (Or just static_cast the int to an Attack enum.)

StackExchange Code Review Q#159980, answer score: 10