Program that guesses your number using bitwise operations

I got the idea for this program from this site.

functions.cpp:

#include 
#include 
#include 

namespace my
{
    int getOneOrZero()
  {
    return (rand() >> 14);   // >> the bitwise operator
  }

// getArray leaves the numbers ( 0-99 ) that have in their bit position        represented by bigFlag the num ( 0 or 1 )

void printNums(int8_t bitFlag, int num)    // num is from getOneOrZero() so it's 1 or 0;
{
    for (int counter = 0; counter > answer;
        std::cin.ignore(32767,'\n');

        if (std::cin.fail())
            std::cin.clear();

        if (answer == 'y' || answer == 'n' || answer == 'r')   // I could use switch but nevermind
            return answer;
    }
}

void turnsPassed(int turns)
{
    std::cout << "\n This is turn " << turns << "\n\n";
}

int swapZeroOrOne(int num)
{
    switch (num)
    {
    case 0 :
        return 1;
    case 1 :
        return 0;
    default:
        std::cout << "\nSwapZeroOrOne ERROR !\n";
        break;
    }
}

int getUpdateForGuessNum(int8_t flag, int num, char answer)
{
    switch (answer)
    {
    case 'n' :
        { int newNum = swapZeroOrOne(num);
          return newNum*flag;
        }
    case 'y' :
        return num*flag;
    case 'r' :
        break;
    default  :
        std::cout << "\n ERROR ! In getUpdateForGuessNum\n";
        break;
    }
  }
}

main.cpp:

```
#include
#include
#include
#include
#include "functions.hpp"
#include "constants.hpp"
#include // for system() commands >.(time(0)));

Reset : // goto !
system("cls");

int guessNum = 0;

// guessing loop !

for (int counter = 0; counter < 7; ++counter)
{
if (counter == 0)
std::cout << "\n Think of a number between 0 to 99\n";

my::turnsPassed(counter + 1); // +1 cause counter starts from 0

int num {my::getOneOrZero()}; // this gives a randomness to the numbers shown each time you run the programm

my::printNums(myVar::bitFlag[counter],num);

char answer { my::getAnswer() };

if (a

Here are some things that may help you improve your code.

Understand type implications

The code includes a bitFlag array that is declared as const int8_t but then contains a value that is 0x80. The problem with that is that when the compiler encounters the constant 0x80, it converts it into an int by default, and so that would be the value 128. However, 128 is not representable in an int8_t. That bit pattern is actually -128 as an int8_t, so I'd recommend either using a different type (such as uint8_t) for the variable or writing -0x80 for the constant which is correct, but a little strange looking.

Know the standard types

This function is in the current code:

int getOneOrZero()
{
    return (rand() >> 14);   // >> the bitwise operator
}

There are two problems with this. The first is that it apparently assumes that an int is 16 bits. However, on my machine, an int is 64 bits. In general, you can't assume that the size of an int is a fixed size. The standard only says (implicitly from the required range) that it must be 16 bits, but it may be larger. The second problem is addressed in the next point.

Consider using a better random number generator

Because you are using a compiler that supports at least C++11, consider using a better random number generator. In particular, instead of rand, you might want to look at std::bernoulli_distribution and friends in the ` header.

Here's one way to rewrite it:

int getOneOrZero()
{
    static std::mt19937 gen{std::random_device{}()};
    static std::bernoulli_distribution bd;
    return bd(gen);   
}

Ensure every control path returns a proper value

The swapZeroOrOne routine returns 1 or 0 under some set of conditions but then doesn't return anything at all otherwise (although it prints an error essage). This is an error because all control paths should return a value. Since it's only used once, and because it can be trivially rewritten, I'd probably replace this:

case 'n' :
    { int newNum = swapZeroOrOne(num);
      return newNum*flag;
    }

with this:

return (1-num)*flag;

Avoid using goto

Having a goto statement in modern C++ is usually a sign of bad design. Better would be to eliminate them entirely -- it makes the code easier to follow and less error-prone. In this code, it's probable that you could use a loop instead:

for (bool playing=true; playing;  )
    // code to play the game
    // ask user if they want to play again
    playing = answer == 'r'; 
}

Don't use system("cls")

There are two reasons not to use system("cls") or system("pause"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named cls or pause, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a seperate functions cls() and pause() and then modify your code to call those functions instead of system. Then rewrite the contents of those functions to do what you want using C++. For example, if your terminal supports ANSI Escape sequences, you could use this:

void cls()
{
    std::cout << "\x1b[2J";
}

Think of the user

If the user were to get an error message that said:


ERROR ! In getUpdateForGuessNum

What use is that unless they also happen to be the author of the code? Instead, for errors that are an indication of a program flaw, I'd use assert or for things that are unusual but still should be accomodated by the program, use an exception.

Use objects

You have a guessNum and counter to support the game and then separate functions printNums and getAnswer, etc. that operate on guessNum. With only a slight syntax change, you would have a real object instead of C-style code written in C++. You could declare a GuessNum object and then printNums, getAnswer, etc. could all be member functions.

Don't use std::endl unless you really need to flush the stream

The difference between std::endl and '\n' is that std::endl actually flushes the stream. This can be a costly operation in terms of processing time, so it's best to get in the habit of only using it when flushing the stream is actually required. It's not for this code.

Omit return 0

When a C or C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main`.

Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments: "I didn't know that." or "That's bad advice!" My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard. For C, since C99; see ISO/IEC 9899:1999 section 5.1.2.2.3:


[...] a return from the initial call

StackExchange Code Review Q#151142, answer score: 15