There are many bad practices in this code. I want to show all I could find and suggest improvements.
- Don't use using namespace std
Pick a naming style:
string yay;
string actyay = "yes";
string nactyay = "no";
string EastEgg = "maybe";
string replay = "replay";
bool Chest_Unlocked;
bool Have_Knife;
bool Dead = !true;
You dont use a consistent naming style here. Use either camel case:
string yay;
string actYay = "yes";
string nActYay = "no";
string eastEgg = "maybe";
string replay = "replay";
bool chestUnlocked;
bool haveKnife;
bool dead = !true;
Or whitespace:
string yay;
string act_yay = "yes";
string n_act_yay = "no";
string east_egg = "maybe";
string replay = "replay";
bool chest_unlocked;
bool have_knife;
bool dead = !true;
Better naming. What is a yay, actyay , nactyay and
EastEgg ?
How about renaming them to userChoice, choiceYes , choiceNo
and choiceWin.
Use false directly. This:
bool Dead = !true;
How about stating directly that dead is false :
bool isDead {false};
Init your Variables. This:
bool Chest_Unlocked;
bool Have_Knife;
This is dangerous. It is not defined if these variables are true or
false at the beginning. You should state your intentions clearly
like this (I also renamed them):
bool chestIsUnlocked{ false }; bool hasKnife{ false };
use constexpr string_view. Change your const variables to this:
constexpr std::string_view choiceYes = "yes"; constexpr
std::string_view choiceNo = "no"; constexpr std::string_view
choiceWin = "maybe"; constexpr std::string_view replay = "replay";
The advantage is, that they are now compile time constants who can't
be modified by accident
Use else if. This:
if (yay == actyay){
Have_Knife = true;
}if (yay == nactyay){
Have_Knife = false;
}if (yay == EastEgg){
goto EasterEgg;
}
Should be this:
if (yay == actyay) {
Have_Knife = true;
}
else if (yay == nactyay) {
Have_Knife = false;
}
else if (yay == EastEgg) {
goto EasterEgg;
}
Reason. If you met one condition you can't reach the others so
there's no reason to still check for them
- Don't use
goto. In several places you use goto to emulate functions.
There's almost never a good reason to use goto at all. Instead use
proper functions (See ending of the post)
Don't use system("pause"). You can replace it with a portable
solution like this:
void pressAnyKeyToContinue()
{
std::cout << "Press any key to continue...\n";
std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
std::cin.get();
}
Handle wrong input. In this:
cout << "Do you pick up the knife?\n";
cin >> yay;
if (yay == actyay){
Have_Knife = true;
}if (yay == nactyay){
Have_Knife = false;
}if (yay == EastEgg){
goto EasterEgg;
}
Besides other problems here. If the user doesn't fullfil any of
these statements you continue your program with invalid input. Try
that out you will run in an exception because Have_Knife also
wan't initialized before. The solution is repeat the user input as
long as it is not valid.
Don't check bools explicit. These ones:
if (Have_Knife == true){
if (Dead == false){
Can be simplified as:
if (hasKnife) {
if (!isDead) {
Use Functions. Currently your program looks like a assembler
program where you use jumps to go to certain parts of the program to
simulate functions. You should divide your program logically into
functions. It will also make it more readable.
Rewritten your code by using some more enhancements I came to this solution (Not perfect but to give you an idea how the code could look):
#include <iostream>
#include <cstdlib>
#include <string>
#include <string_view>
#include <limits>
namespace game {
void clearScreen()
{
#ifdef _WIN32
std::system("cls");
#else
std::system("clear"); // most other systems use this
#endif
}
void pressAnyKeyToContinue()
{
std::cout << "Press any key to continue...\n";
std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
std::cin.get();
}
void printWin()
{
clearScreen();
std::cout << "You Win!";
pressAnyKeyToContinue();
}
struct Player {
bool hasKnife{ false };
bool wins{ false };
};
void userChoicePickUpKnifeOrWin(Player& player,
std::string_view choiceYes,
std::string_view choiceNo,
std::string_view choiceWin)
{
std::string userChoice;
for (;;) {
clearScreen();
std::cout << "Do you pick up the knife?\n";
std::cin >> userChoice;
if (userChoice == choiceYes) {
player.hasKnife = true;
return;
}
else if (userChoice == choiceNo) {
player.hasKnife = false;
return;
}
else if (userChoice == choiceWin) {
player.wins = true;
return;
}
}
}
bool userChoiceUnlockChest(std::string_view choiceYes,
std::string_view choiceNo)
{
std::string userChoice;
for (;;) {
clearScreen();
std::cout << "Did you unlock the chest?\n";
std::cin >> userChoice;
if (userChoice == choiceYes) {
return true;
}
else if (userChoice == choiceNo) {
return false;
}
}
}
void printSnakeIsInChest()
{
clearScreen();
std::cout << "Inside the chest is a big fat snake :O\n";
pressAnyKeyToContinue();
}
void printDead()
{
std::cout << "You arise to heaven and meet God the Almighty\n"
<< "God says unto you...\n"
<< " Thou hast lost the game of life and death"
<< "because of this you have come back unto my realm."
<< "I give you a choice that will only come once in a lifetime.\n"
<< "Would you like to replay dat crappy little game or have\n"
<< "Fun in Heaven?\n";
}
bool choiceReplay(std::string_view replay)
{
std::string userChoice;
std::cin >> userChoice;
return userChoice == replay;
}
bool playGame()
{
constexpr std::string_view choiceYes = "yes";
constexpr std::string_view choiceNo = "no";
constexpr std::string_view choiceWin = "maybe";
constexpr std::string_view replay = "replay";
Player player;
userChoicePickUpKnifeOrWin(player, choiceYes, choiceNo, choiceWin);
if (player.wins) {
return false;
}
if (userChoiceUnlockChest(choiceYes, choiceNo)) {
printSnakeIsInChest();
}
else {
clearScreen();
std::cout << "You Lose\n";
printDead();
return choiceReplay(replay);
}
if (player.hasKnife) {
clearScreen();
std::cout << "Since you have dat handy knife you kill dat snakey\n";
pressAnyKeyToContinue();
return false;
}
clearScreen();
std::cout << "YOU DIE HAHAHAHAHAHAHA\n";
printDead();
return choiceReplay(replay);
}
}
int main()
{
while (game::playGame()) {
;
}
game::printWin();
}
