HiveBrain v1.2.0
Get Started
← Back to all entries
patterncModerate

Binary numbers game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
numbersgamebinary

Problem

I wrote a little command line nerdy game, which helps you learn the basics of binary numbers. I would be happy to hear your comments:

```
#include
#include
#include
#include
#define CHARSIZE 10

void printbitssimple(int);
void createbitshape(int, char[]);
int guess_num_from_binary(void);
int guess_binary_from_num(void);

int main(void)
{

int play_on = 1;
int ans;
char response[CHARSIZE];

printf("\n\t The BitShape Game \n");
while(play_on)
{
printf("Enter a choice, which game you would like to play:\n");
printf("1 - Guess a number from it's bit shape ...\n");
printf("2 - Given an integer type in it's bit shape ...\n");
printf("3 - Exit the game\n");
fgets(response, CHARSIZE, stdin);
printf("Printing s: %s\n", response);
switch(atoi(response)){

case 1:
printf("Guess a number from it's bit shape ...\n");
/ if correct guess_num_from_binary returns 0 /
ans = guess_num_from_binary();
if (! ans) {
printf("\nThat's Correct!\n");
} else {
printf("\nThat's Wrong! The correct answer is %d\n", ans);
}
break;
case 2:
printf("Play version 2\n");
guess_binary_from_num();
break;
case 3:
printf("Good bye\n");
play_on = 0;
break;
default:
printf("Did not understand your input...\n");
break;

}
}
return 0;
}

/ Print n as a binary number /
void printbitssimple(int n)
{
unsigned int i;
/ printf("The sizeof of n is %ld\n", sizeof(n));/
i = 1 0)
{
if (n & i) / check if any of the bits of n is not 0 ./
printf("1");
else
printf("0");
i >>= 1;
}
}

/*similar to printbitssimple, but instead

Solution

Things you did well:

-
When you didn't take in parameters to a function, you declared them void.

-
Good use of comments.

Things you could improve:

Refactoring:

-
You can simplify your function printbitssimple(int n).

void printbitssimple(int n) 
{
    unsigned int i;
    /* printf("The sizeof of n is %ld\n", sizeof(n));*/
    i = 1 0) 
    {
        if (n & i)  /* check if any of the bits of n is not 0 .*/
            printf("1");
        else
            printf("0");
        i >>= 1;
    }
}


Utilize the power of a for loop and the ternary conditional expression. Note that my method will prepend some additional 0's to the beginning of the output.

void printbitssimple(int n)
{
    for (unsigned bit = 1u >= 1)
    {
        putchar((n & bit) ? '1' : '0');
    }
}


-
You can do the same with your method createbitshape().

void createbitshape(int n, char bitshp[]){
    unsigned int i;
    /* printf("The sizeof of n is %ld\n", sizeof(n));*/
    int c = 0;
    i = 1 0) 
    {
        if (n & i)  /* check if any of the bits of n is not 0 .*/
        {
            //printf("1");
            bitshp[c] = '1';
        }
        else
        {
            //printf("0");
            bitshp[c] = '0';
        }
        i >>= 1;
    c=c+1;
    }
    bitshp[c] = '\0';
}


Here's my implementation.

void createbitshape(int n, char bitshp[])
{
    int c = 0;
    for (unsigned bit = 1u >= 1, c++)
    {
        bitshp[c] = ((n & bit) ? '1' : '0');
    }
    bitshp[c] = '\0';
}


Syntax/Styling:

-
Use puts() instead of printf() when you aren't formatting a string.

printf("Here is the number:\n");


puts("Here is the number:");


-
You have some superfluous space in your code.

int main(void) 
{

int play_on = 1;
int ans;
char response[CHARSIZE];

printf("\n\t The BitShape Game \n");


I am all for space, but to an extent. Use only one space between your separate "blocks" of code.

Variables:

-
Always initialize variables.

int random_num;
char ans[20];


Non-static variables (local variables) are indeterminate. Reading them prior to assigning a value results in undefined behavior.

int random_num = 0;
char ans[20] = {0};

Code Snippets

void printbitssimple(int n) 
{
    unsigned int i;
    /* printf("The sizeof of n is %ld\n", sizeof(n));*/
    i = 1<<(sizeof(n) * 2 - 1);
    while (i > 0) 
    {
        if (n & i)  /* check if any of the bits of n is not 0 .*/
            printf("1");
        else
            printf("0");
        i >>= 1;
    }
}
void printbitssimple(int n)
{
    for (unsigned bit = 1u << 31; bit != 0; bit >>= 1)
    {
        putchar((n & bit) ? '1' : '0');
    }
}
void createbitshape(int n, char bitshp[]){
    unsigned int i;
    /* printf("The sizeof of n is %ld\n", sizeof(n));*/
    int c = 0;
    i = 1<<(sizeof(n) * 2 - 1);

    while (i > 0) 
    {
        if (n & i)  /* check if any of the bits of n is not 0 .*/
        {
            //printf("1");
            bitshp[c] = '1';
        }
        else
        {
            //printf("0");
            bitshp[c] = '0';
        }
        i >>= 1;
    c=c+1;
    }
    bitshp[c] = '\0';
}
void createbitshape(int n, char bitshp[])
{
    int c = 0;
    for (unsigned bit = 1u << 31; bit != 0; bit >>= 1, c++)
    {
        bitshp[c] = ((n & bit) ? '1' : '0');
    }
    bitshp[c] = '\0';
}
printf("Here is the number:\n");

Context

StackExchange Code Review Q#43238, answer score: 10

Revisions (0)

No revisions yet.