5
\$\begingroup\$

I made this simple game code. Can you please give me some advice on how I can improve it?

#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();
}
\$\endgroup\$
  • \$\begingroup\$ Tabs don't translate well to the web. Please format your code so it is easy to read. \$\endgroup\$ – Martin York Mar 9 '13 at 17:14
  • \$\begingroup\$ easier to read now ? \$\endgroup\$ – user2129387 Mar 9 '13 at 17:23
  • \$\begingroup\$ 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. \$\endgroup\$ – Martin York Mar 9 '13 at 17:39
  • 1
    \$\begingroup\$ Why did you include windows.h? \$\endgroup\$ – luiscubal Mar 10 '13 at 0:25
10
\$\begingroup\$

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);
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ and by the way the game is meant to be 10 guesses otherwise it wont be fun... \$\endgroup\$ – user2129387 Mar 9 '13 at 18:32
  • \$\begingroup\$ Even so. I still prefer my bit at the end. It look tidier with one print statement. \$\endgroup\$ – Martin York Mar 9 '13 at 18:59
  • \$\begingroup\$ Also, gess->guess. \$\endgroup\$ – luiscubal Mar 10 '13 at 0:03
  • \$\begingroup\$ @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. \$\endgroup\$ – Martin York Mar 10 '13 at 0:14
  • \$\begingroup\$ shouldn't it be gess < 10 || gess >= 20? As it stands, this will never be true. \$\endgroup\$ – beatgammit Mar 27 '13 at 8:39
3
\$\begingroup\$

As mentioned in the comments, there's no need for <windows.h> here. For portability reasons, don't include it unless you're actually using code from it. It has nothing to do with running the program on a Windows system, if that's what you were thinking.

Also, your unformatted output statements (no %) can use puts() instead of printf(). The former also provides a \n at the end of the output.

| improve this answer | |
\$\endgroup\$
-1
\$\begingroup\$
#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
| improve this answer | |
\$\endgroup\$
  • 2
    \$\begingroup\$ Your suggestions are hard to read this way, as comments embedded in the code. I suggest to take the comments outside, see how other answerers do it. It will be a lot more readable, and that's the aim, isn't it? Giving answers that are easy to read, just like code should be easy to read, accessible. \$\endgroup\$ – janos Dec 12 '15 at 18:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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