is there a better way to write this function?

[wuubb]

G'day mates!

I have been working on a "games" program in C++ that includes several games including a rock, paper, scissors. I have a header file, where I have declared all of the functions I use in the games, as well as a .cpp file which includes the definitions. My question is, on line 104 (in Games.cpp), where I begin defining testCpuPlayRPS function, I take the return value of the previous function (testYourPlayRPS) and use that in the if statement. However, the string parameter in the first "if" statement (yourPlay), I have to declare that in the header file cause im not sure what I should put there. I want both "yourPlay" and "theirPlay" (the parameter of the other function) to be what is eventually inputted from the console. I feel like there should be a better way to do this than having to use that intermediary string variable (yourPlay). Any suggestions?

And if there is anything else wrong with my program so far, please do critique, I have been having difficulty understanding how to properly use header files and creating your own libraries and such, so I hope I am doing it right.

Thanks, any help is appreciated!
Cheers!

//Games.h
#ifndef GAMES_H
#define GAMES_H

#include <string>


namespace games
{

	std::string yourPlay;

	//functions
	int genRandNum();

	std::string reverseString(const std::string& oldString);

	int testHiLo(int guess, int answer);

	bool testPalindrome(std::string palindrome);

	int testYourPlayRPS(std::string theirPlay);

	int testCpuPlayRPS(int cpuPlay);

	int testSlotMachine(int slotNum1, int slotNum2, int slotNum3);
}

#endif

//Games.cpp
#include "Games.h"
#include <algorithm>
#include <boost\algorithm\string\predicate.hpp>

using namespace std;

namespace games
{
	int genRandNum(int maxRand)
	{
		int result = (rand() % maxRand);
		return result;
	}

	string reverseString(const string& oldString)
	{
		string newString(oldString);
		reverse(newString.begin(), newString.end());
		return newString;
	}

	int testHiLo(int guess, int answer)
	{

		if (guess==answer)
		{
			return 0;
		}
		else if (guess>answer)
		{
			return 1;
		}
		else if (guess < answer)
		{
			return 2;
		}
	}

	bool testPalindrome(string palindrome)
	{
		if (boost::iequals(palindrome, reverseString(palindrome)))
		{
			return true;
		}
		else
		{
			return false;
		}
	}

	int testYourPlayRPS(string theirPlay)
	{
		if ((boost::iequals(theirPlay, "rock")) || (boost::iequals(theirPlay, "1")))
		{
			return 1;
		}
		else if ((boost::iequals(theirPlay, "paper")) || (boost::iequals(theirPlay, "2")))
		{
			return 2;
		}
		else if ((boost::iequals(theirPlay, "scissors")) || (boost::iequals(theirPlay, "3")))
		{
			return 3;
		}
		else
		{
			return 0;
		}
	}

	int testCpuPlayRPS(int cpuPlay)
	{
		if (testYourPlayRPS(yourPlay)==1)
		{
			if (cpuPlay==1)
			{
				return 11;
			}
			else if (cpuPlay==2)
			{
				return 12;
			}
			else if (cpuPlay==3)
			{
				return 13;
			}
			else
			{
				return 0;
			}
		}
		else if (testYourPlayRPS(yourPlay) == 2)
		{
			if (cpuPlay == 1)
			{
				return 21;
			}
			else if (cpuPlay == 2)
			{
				return 22;
			}
			else if (cpuPlay == 3)
			{
				return 23;
			}
			else
			{
				return 0;
			}
		}
		else if (testYourPlayRPS(yourPlay) == 3)
		{
			if (cpuPlay == 1)
			{
				return 31;
			}
			else if (cpuPlay == 2)
			{
				return 32;
			}
			else if (cpuPlay == 3)
			{
				return 33;
			}
			else
			{
				return 0;
			}
		}
		else if (testYourPlayRPS(yourPlay) == 0)
		{
			return 0;
		}
	}

	int testSlotMachine(int slotNum1, int slotNum2, int slotNum3)
	{
		if ((slotNum1 == slotNum2) && (slotNum1 == slotNum3))
		{
			return 3;
		}
		else if ((slotNum1 == slotNum2) || (slotNum2 == slotNum3) || (slotNum1 == slotNum3))
		{
			return 2;
		}
		else
		{
			return 1;
		}
	}


}

[Duthomhas]

A number of thoughts.

A palindrome typically excludes non-alphabetic characters. For example: "A man, a plan, a canal: Panama!"

Nice use of the Boost predicate!

Anytime you find yourself using a whole bunch of if..else statements, there is almost always a better way. Either use a switch, or use a lookup table. Sometimes you can even just compute the answer, like in testCpuPlayRPS():

	int testCpuPlayRPS(int cpuPlay)
	{
		int a = testYouPlayRPS(yourPlay);
		if (a == 0) return 0;

		// The CPU shouldn't be allowed to make a mistake.
		// Therefore, at this point we should be able to assume 
		// that we have a good value in 'cpuPlay'.

		return (a * 10) + cpuPlay;
	}

You can use a little logic to reduce the testSlotMachine() too.

Your code looks clean.

[wuubb]

Thanks mate, I really appreciate it!

I'll definitely "look up" what a lookup table is and see if I can implement it somewhere. However, I don't quite understand how the code you posted replaces the if, else. I think I need a little more clarification if thats not too much.

Thank you for the response though,
Cheers!