Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I made this simple game code. Can you please give me some advice about how can I improve it ? I thought about making the screen blue and the letters yellow can someone explain me how to do that ?

    #include<stdio.h>
    #include<stdlib.h>
    #include<conio.h>
    #include <time.h>
    #include <windows.h>

    void main ()
    {
        printf("\t\tWelcome to our gessing game\n\n\n");
        printf("the computer choose a random number between 1-99, try to gess the random number...");

        srand(time(NULL));

        int gess,i=0,found=0,  r = rand()%100;

        while (i<10 && found==0)
        {
            printf("\n My gess is:\t");
            scanf("%d",&gess);
            if (gess==r)
            {
                found=1;
                i++;
            }
            else if(gess>r)
            {
                printf("\n Too big\n");
                i++;
            }
            else if(gess<r)
            {
                printf("\n Too small\n");
                i++;
            }
        }
        if (found==1&&gess==1)
            printf("\n Very good the number is %d this is your %dst gess",gess,i);
        else if(found==1&&gess==2)
            printf("\n very good the number is %d this is your %dnd gess",gess,i);
        else if(found==1&&gess==3)
            printf("\n very good the number is %d this is your %drd gess",gess,i);
        else if(found==1&&gess!=1&&gess!=2&&gess!=3)
            printf("\n very good the number is %d this is your %dth gess",gess,i);
        else
            printf("\n Never mind try again");
        getch();
    }
share|improve this question
Tabs don't translate well to the web. Please format your code so it is easy to read. – Loki Astari Mar 9 at 17:14
easier to read now ? – user2129387 Mar 9 at 17:23
I already formatted so it was easy to read. You just overwrote it. But if you are going to make an effort please look at it and make sure it looks good (you seem to have put all the if against the left side). Its hard to read code at the best of times try not to make it harder with bad formatting. – Loki Astari Mar 9 at 17:39
Why did you include windows.h? – luiscubal Mar 10 at 0:25

2 Answers

One variable per line please.

int gess,i=0,found=0,  r = rand()%100;

This makes the code hard to read. There is also one corner case (with pointers) were this will not work as expected for beginners. As a result this is usually banned in most companies coding standards.

You are using found as a boolean variable.
Treat it as such. use TRUE/FALSE.

I would change the while() into a for(;;) loop. Then you can remove all the increments.

while (i<10 && found==0)

--

for(int i = 0; i < 10 && !found; ++i)

White space is your friend:

if (found==1&&gess==1)  // This is hard to read and will get worse with more conditions.

if (found && (guess == 1)) // much more readable
//  ^^^^^  boolean use it as such.

Your "st", "nd", "rd", "th" is not correct.

10-19    => th
x1       => st
x2       => nd
x3       => rd
x[4-9]   => th

Also you print basically the same string every time on success. So we not have one print statement and just separate the logic for getting the ending. I would change the logic:

if (!found)
{
    printf("\n Never mind try again");
}
else
{
    char const* ext = "th";
    if ((gess < 10) || (gess >= 20))  // 10->19 already set
    {
        int end = gess % 10;
        if      (end == 1) { ext = "st";}
        else if (end == 2) { ext = "nd";}
        else if (end == 3) { ext = "rd";} 
    }

    printf("\n Very good the number is %d this is your %d%s gess", gess, i, ext);
}
share|improve this answer
thanks for helping :) – user2129387 Mar 9 at 18:25
and by the way the game is meant to be 10 guesses otherwise it wont be fun... – user2129387 Mar 9 at 18:32
Even so. I still prefer my bit at the end. It look tidier with one print statement. – Loki Astari Mar 9 at 18:59
Also, gess->guess. – luiscubal Mar 10 at 0:03
@luiscubal: I don't correct spelling of text output. That;s the job of a manager. It could be another language for all I know. – Loki Astari Mar 10 at 0:14
show 2 more comments
#include<stdio.h>
#include<stdlib.h>
#include <time.h>



int main()  // Main always return int ...ALWAYS...
    {
        printf("\t\tWelcome to our gessing game\n\n\n");   // This could be device specific. If i run it on a minimized screen it might look akward

        /*
            maybe, call a function like take_device_width() then use the value returned to
            print the welcome message
        */
        printf("the computer choose a random number between 1-99, try to gess the random number...");

        /* You could write something like this

            #define MIN_NUM 1
            #define MAX_NUM 99

            printf(""The Computer chooses a random number between %d and %d. \
                   You have to guess it using minumun number of guesses\n", MIN_NUM, MAX_NUM);

            So that when ever you want to change the values for number range you can just
            change it once at pre processor macro.

        */

        srand(time(NULL));     // srand((unsigned) time(NULL));

        /* Dont use bad spellings and always indent code cleanly: i,e int guess, i = 0, found = 0*/

        int gess,i=0,found=0,  r = rand()%100;

        /* INstead of saying that number of guesses is i, you can name it as num_of_guesses. So that next time
           when you look at code you will know what num_of_guesses does . Similarly for r.
        */

         /* Instead of using magic number 10, you could write it as a macro:

            #define MAX_ATTEMPTS 10

        */


        while (i<10 && found==0)        // indent it here: while(num_of_guesses < MAX_ATTEMPT && found == 0)  
        {
            printf("\n My gess is:\t"); // no need of \t here
            scanf("%d",&gess);

            /* Indent it. After a block that does something add a comment of what next block does nad write that code */
            if (gess==r)
            {
                found=1;    // instead of found we can do this

                /* if(guess == r)
                   {
                        num_of_guesses++;
                       printf("You guessed the correct answer in %d guesses", num_of_guesses);
                       exit(0);
                   }

                   will avoid checking for while(condition) even after guessing correct answer
                */
                i++;
            }

            else if(gess>r)
            {
                printf("\n Too big\n");
                i++;
            }
            else                            // else is enough here
            {
                printf("\n Too small\n");
                i++;
            }

            /* most of the newline char are unneccesary */
        }

        /* Indent here */

        /* This looks daunting. What are you trying to do here. Why so many conditions */

        if (found==1&&gess==1)
            printf("\n Very good the number is %d this is your %dst gess",gess,i);
        else if(found==1&&gess==2)
            printf("\n very good the number is %d this is your %dnd gess",gess,i);
        else if(found==1&&gess==3)
            printf("\n very good the number is %d this is your %drd gess",gess,i);
        else if(found==1&&gess!=1&&gess!=2&&gess!=3)
            printf("\n very good the number is %d this is your %dth gess",gess,i);
        else
            printf("\n Never mind try again");
        getchar();

        /* it could be 
         * if(num_of_guesses >= MAX_ATTEMPTS)
           {
               printf("You couldn't guess it in suggested attempts. Sorry!");
               exit(1);

               you can even add a yes or no condition to play again

           }

        */
       return 0;
    }// end main
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.