Finished Tic Tac Toe in C - Feedback on my source?

Finished Tic Tac Toe in C - Feedback on my source? - Printable Version

+- iDevGames Forums (http://www.idevgames.com/forums)
+-- Forum: Development Zone (/forum-3.html)
+--- Forum: Game Programming Fundamentals (/forum-7.html)
+--- Thread: Finished Tic Tac Toe in C - Feedback on my source? (/thread-4231.html)

Pages: 1 2

Finished Tic Tac Toe in C - Feedback on my source? - Achithyn - May 29, 2006 04:43 PM

Greetings everyone,
I would first like to thank everyone who has been helping me get back into programming. All the book suggestions, project suggestions, etc. has really helped me define a good path to tread down.

Well, I programmed my first completed C game, which is Tic Tac Toe. I know everyone is different, and thus, programs differently, though I feel like everyone will laugh when they see my source... lol. It's 575 lines, which seems long for a tic tac toe game.

I made the AI beatable, or it was at one point. Haven't been able to beat it for a while. I tried to put quality into the game, even though it runs in command line. There may be a bug here and there, though nothing that I have found over the last few look overs.

Well, I just wanted to get some feedback on my code. I just started to learn C, so the code may not look very professional. >< Any suggestions on how to make the code better would be great - it would help me learn a great deal.

Thanks for your time!

Source:
http://www.arcane-artistry.net/programming/tictactoe.zip

~Achi

PS. This is the first time I have zipped my source and presented it for download - let me know if I did it right.

PPS. I hope this is the right place to post this - I have the hardest time figuring out what goes where... >< Let me know if I posted in the wrong area, and I won't make the mistake again.

Finished Tic Tac Toe in C - Feedback on my source? - Blacktiger - May 29, 2006 11:27 PM

DISCLAIMER:All of the following are suggestions based on my personal programming style. Take what suits you.

Opening the source seems to work fine. I'll look closer at the code later, but it seems pretty good for your first game. One thing that would probably help shorten your code and perhaps make things a bit more clear would be to store the tic-tac-toe board in a matrix instead of three arrays.

Instead of this:

Code:

    char topBoard[3][2] = { "1", "2", "3" };

    char midBoard[3][2] = { "4", "5", "6" };

    char botBoard[3][2] = { "7", "8", "9" };

Try this:

Code:

char board[3][3];

Your setup board function would change to a display board function:

Code:

void displayBoard()

{

    printf( "\n/------Play Against-------\\\n" );

    printf( ".-----------CPU-----------.\n" );

    printf( "/\\       /-------\\       /\\\n" );

    printf( "-- TIC --| %s %s %s |-- TIC --\n", board[0][0], board[0][1], board[0][2] );

    printf( "|| TAC   | %s %s %s |   TAC ||\n", board[1][0], board[1][1], board[1][2] );

    printf( "-- TOE --| %s %s %s |-- TOE --\n", board[2][0], board[2][1], board[2][2] );

    printf( "\\/       \\-------/       \\/\n" );

    printf( "\t%s = X || CPU = O\n\n", userName );

}

You would need a new setup board function that took no arguments
(or possibly a board argument):

Code:

void setupBoard()

{

    board = { '1', '2', '3',

              '4', '5', '6',

              '7', '8', '9'

            };

}

Then to test whether a cats game has occurred you can use two loops.

Code:

void checkCAT()

{

    bool cats = true;

    for(int row = 0; row < 3; row++)

    {

        for(int col = 0; col < 3; col++)

        {

            if(board[row][col] != 'X' && board[row][col] != 'O')

                // This space is unused

                cats = false;

        }

    }

}

(I can actually think of a couple other ways to do this. For example, you could code the cats game check to stop once it found spot that wasn't taken yet.)

I also noticed that you put the keyword void in functions that have no arguments. While that is perfectly valid C, why not just remove the keyword completely as its less typing and is just as clear.

I subscribe to the idea that code should be written in such a way that its intent is obvious, but that's not always possible or desirable. I do suggest that whenever you create a function you put not only a description of what the function does above it, but also what assumptions are made about the state of the program. It will help a lot when you have to go back and debug.

Finished Tic Tac Toe in C - Feedback on my source? - ThemsAllTook - May 30, 2006 06:20 AM

Indeed, pretty nice for a first try. It's really easy to read through the code and see what's going on. There were a few formatting/indenting issues, but nothing stuck out to me as particularly egregious.

In addition to Blacktiger's post, one suggestion I'd make is to try to use loops whenever they can reduce code duplication. Certain functions in your code (PlayerTurn, CheckWin, and CPUAI in particular) have what looks like an unrolled loop written in them - largely the same block of code duplicated several times with different values in each. If you need to change the logic, you'd have to make your change 9 times instead of just one. You also have a bit more potential for error than if your logic was all in one place.

With practice, you'll undoubtedly figure out what works best on your own. It definitely looks like you're on the right track. Keep it up!

Finished Tic Tac Toe in C - Feedback on my source? - unknown - May 30, 2006 06:48 AM

Totaly super!
Very finished, and a formidable computer opponent.

Finished Tic Tac Toe in C - Feedback on my source? - Zekaric - May 30, 2006 01:54 PM

Code:

void displayBoard()

{

    printf( "\n/------Play Against-------\\\n" );

    printf( ".-----------CPU-----------.\n" );

    printf( "/\\       /-------\\       /\\\n" );

    printf( "-- TIC --| %s %s %s |-- TIC --\n", board[0][0], board[0][1], board[0][2] );

    printf( "|| TAC   | %s %s %s |   TAC ||\n", board[1][0], board[1][1], board[1][2] );

    printf( "-- TOE --| %s %s %s |-- TOE --\n", board[2][0], board[2][1], board[2][2] );

    printf( "\\/       \\-------/       \\/\n" );

    printf( "\t%s = X || CPU = O\n\n", userName );

}

I'm not sure how many people know this but this can be rewritten to be just one call instead of the 8 that is used.

Code:

void displayBoard()

{

    printf(

        "\n"  /* Strings get concatenated by the compiler. */

        "/------Play Against-------\\\n"

        ".-----------CPU-----------.\n"

        "/\\       /-------\\       /\\\n"

        "-- TIC --| %s %s %s |-- TIC --\n"

        "|| TAC   | %s %s %s |   TAC ||\n"

        "-- TOE --| %s %s %s |-- TOE --\n"

        "\\/       \\-------/       \\/\n"

        "\t%s = X || CPU = O\n\n",

        board[0][0], board[0][1], board[0][2], 

        board[1][0], board[1][1], board[1][2],

        board[2][0], board[2][1], board[2][2],

        userName);

}

Sorry, I haven't looked at the code yet. Maybe I'll get a chance tonight.

Finished Tic Tac Toe in C - Feedback on my source? - imikedaman - May 30, 2006 02:15 PM

I think he did that just to have the text lined up nicely in the source, although technically your way is lined up too. Meh...

Finished Tic Tac Toe in C - Feedback on my source? - OneSadCookie - May 30, 2006 03:16 PM

Blacktiger Wrote:I also noticed that you put the keyword void in functions that have no arguments. While that is perfectly valid C, why not just remove the keyword completely as its less typing and is just as clear.

That would be incorrect. This is an area of difference between C and C++:

Code:

// C

void foo(void); // foo takes no arguments

void foo(...); // foo takes a variable number of arguments

void foo(); // foo takes a variable number of arguments (bad style)

Code:

// C++

void foo(void); // foo takes no arguments (bad style)

void foo(...); // foo takes a variable number of arguments

void foo(); // foo takes no arguments

Finished Tic Tac Toe in C - Feedback on my source? - Zekaric - May 30, 2006 03:26 PM

Just to nit pick. For variable argument list functions you need at least one parameter. Unless that has changed in C++ or a more modern C.

Code:

void foo(int FirstArg, ...);

This is because the functions/macros to get to the variable portion needs something to start with. Nit pick mode off.

Finished Tic Tac Toe in C - Feedback on my source? - OneSadCookie - May 30, 2006 04:42 PM

You do need at least one argument if you want to use stdarg to extract the values, but this is (unfortunately) perfectly legal:

Code:

// header

void foo(...);

Code:

//implementation

void foo(int arg0, double arg2)

{

    // ...

}

This will work fine if the function is called as

Code:

foo(3, 3.0);

but blow up horribly somehow if it's called as

Code:

foo(3, 3);

Make sure your prototypes match your definitions

Finished Tic Tac Toe in C - Feedback on my source? - Achithyn - May 30, 2006 07:54 PM

Greetings all,
Thank you all for your feedback and suggestions.

I'm going to comment on everything up to post #7, since you all kind of lose me at that point. lol. I added the extra ( void ); only because that is how my book does it.

It doesn't take long to type it in, so no worries there.

@Blacktiger
I didn't know I could setup the board with one char array. If I did, I would have done that. >< I thought that setupBoard[3][3] would be three lines of text that were three characters long, with the trailing \0. So, if I called setupBoard[3][0], would it need an 'end of line' marker \0? [3][3] is setting up to reference 9 characters, though only has three \0's - I guess that is what I feel is the hardest for me to understand... what \0 actually does, and when you need to pay attention to it, and when you don't.

I'm not sure, though would the following code actually work? Is 'board' a pointer? I just thought that you couldn't assign char's like that, less it was when the char was defined and initialized. So, something like userName = "Foo"; wouldn't work, right?

Code:

void setupBoard(

{

    board = { '1', '2', '3',

              '4', '5', '6',

              '7', '8', '9'

            };

}

I think your cat loop is very well thought out. However, it doesn't save too many lines of code... and (to me)

can be kind of confusing to follow instead of what I had:

Code:

void CheckCat( void )

{

        if    ((strcmp(topBoard[0], "1") != 0 ) &&

            (strcmp(topBoard[1], "2") != 0 ) &&

            (strcmp(topBoard[2], "3") != 0 ) &&

            (strcmp(midBoard[0], "4") != 0 ) &&

            (strcmp(midBoard[1], "5") != 0 ) &&

            (strcmp(midBoard[2], "6") != 0 ) &&

            (strcmp(botBoard[0], "7") != 0 ) &&

            (strcmp(botBoard[1], "8") != 0 ) &&

            (strcmp(botBoard[2], "9") != 0 ))

        {

            printf( "CATS Game!\n");

            gameStatus = false;

        }

}

I can see how a loop, such as what you had, could work better in some cases though. I am sure that later on, as I grow more and more used to syntax and the use of it, the loop in your example would be more ideal. Honestly, I don't really understand why my code works... can someone win by taking the last place on a tic tac toe board? Or are all victories and defeats determined before all places are taken up? If the last move was played in the last place available on the board, and that play would have constituted as a win, then would the program show: Winner! 'or' Cats! (?)

@ThemsAllTook
Thank you for your feedback.

I'm glad the code was easy to read... perhaps that is because I need it to be easy to read for me to understand how it works.

As for loops, I agree. I need to try to understand how I can use loops more to benefit my code. I know all about loops, though I don't know how to effectively and efficiently use them. I'll work on that. : )

@Unknown
Lol, I'm glad you liked it.

It's funny, I have a member on my forum named Unknown... or was before I reset the site a few months ago. Perhaps your name is common on the net... less you enjoy creative writing?

@Zekaric
I did not know I could have done all that with use one printf function... blah! I think your revision of the code is very nice. Next time I am tempted with using several printf's in a row, I will remember what you said - and your good example. Thanks!

@imikedaman
Lol, unfortunately I did not know about the printf trick. ><

Well, thank you all for your feedback! It was really helpful. I keep coming here and asking for help - is there anything I can do to help the community? Well, let me know! Have a good one!

~Achi

Finished Tic Tac Toe in C - Feedback on my source? - unknown - May 30, 2006 08:14 PM

haha Achithyn, I love creative writing! I only read it though, My english is not good enough

Finished Tic Tac Toe in C - Feedback on my source? - Achithyn - May 30, 2006 08:41 PM

Ah, sorry to hear... >< Though, if you are a programmer, I am sure you can do anything.

A good grammar book and a lot of writing will do wonders.

Do as you want in life, for you can only live it once - that's kind of my motto at the moment...

~Achi

Finished Tic Tac Toe in C - Feedback on my source? - Blacktiger - May 30, 2006 09:54 PM

OneSadCookie Wrote:That would be incorrect. This is an area of difference between C and C++:

Code:

// C void foo(void); // foo takes no arguments void foo(...); // foo takes a variable number of arguments void foo(); // foo takes a variable number of arguments (bad style)

Code:

// C++ void foo(void); // foo takes no arguments (bad style) void foo(...); // foo takes a variable number of arguments void foo(); // foo takes no arguments

Your code ninja skills got me there!

Finished Tic Tac Toe in C - Feedback on my source? - Zekaric - May 30, 2006 10:23 PM

You wrote...

Code:

char topBoard[3][2] = { "1", "2", "3" };

char midBoard[3][2] = { "4", "5", "6" };

char botBoard[3][2] = { "7", "8", "9" };

...

if ( (marker == 1) && (strcmp(topBoard[0], "1") == 0))

{

...

As someone pointed out this is a bit clumsy. You could have done...

Code:

char topBoard[] = "123";

char midBoard[] = "456";

char botBoard[] = "789";

...

if ((marker == 1) && (topBoard[0] == '1'))

{

...

strcmp is slow. This way we are dealing with 3 arrays of chars. Comparing char values which in short is comparing very small integers instead of strings. Much faster (not that it matters here but you avoid the strcmp() function call which can trip people up.) A better way would be to use one array as someone else pointed out.

Code:

char board[] = "123456789";

...

if (marker == 1 && board[0] == '1')

{

...

Similarly the players could be changed to just be chars... char playerOne = 'X';

You wrote...

Code:

if ( (marker == 1) && (strcmp(topBoard[0], "1") == 0))

...

      if ( (marker == 2) && (strcmp(topBoard[1], "2") == 0))

...

      if ( (marker == 3) && (strcmp(topBoard[2], "3") == 0))

...

      if ( (marker == 4) && (strcmp(midBoard[0], "4") == 0))

...

      if ( (marker == 5) && (strcmp(midBoard[1], "5") == 0))

...

      if ( (marker == 6) && (strcmp(midBoard[2], "6") == 0))

...

      if ( (marker == 7) && (strcmp(botBoard[0], "7") == 0))

...

      if ( (marker == 8) && (strcmp(botBoard[1], "8") == 0))

...

      if ( (marker == 9) && (strcmp(botBoard[2], "9") == 0))

      {

         strcpy( botBoard[2], "X" );

         return 0;

      }

Which can be compacted down to just...(using the last code in the above section)

Code:

if (1 <= marker && marker <= 9 && board[marker - 1] == '0' + marker) 

{

   board[marker - 1] = 'X';

   return 0;

}

Similarly for the replay() function

Code:

for (i = 0; i < 9; i++) {

   board[i] = '1' + i;

}

For the most part you shouldn't have too much trouble but usually when passing in arrays of chars...

Code:

/* Compiler may generate warnings if you up the warning level.*/

void function(char array[20]);

/* No warnings */

void function(char *array);

I'd stay away from "Magic Numbers". Like return 0; which was used above. Start using...

Code:

#define FALSE 0

#define TRUE 1

And enums to avoid them. Basically give them meaning for future sanity sake.

You can cause a buffer overflow if you type in a name that is larger than 49 characters but that is just being fussy.

Finished Tic Tac Toe in C - Feedback on my source? - Blacktiger - May 30, 2006 10:34 PM

Achithyn Wrote:I didn't know I could setup the board with one char array. If I did, I would have done that. >< I thought that setupBoard[3][3] would be three lines of text that were three characters long, with the trailing \0. So, if I called setupBoard[3][0], would it need an 'end of line' marker \0? [3][3] is setting up to reference 9 characters, though only has three \0's - I guess that is what I feel is the hardest for me to understand... what \0 actually does, and when you need to pay attention to it, and when you don't.

Ah, let me help you with the \0 thing. First, what I created was a character array (or an array of characters depending on how you want to think of it...). Since we know what the size of the board is at all times, we can just use the size to correctly index any position. \0 is the null character and is used when working with character strings (c-strings). To create a character you use single quotes ('c') and you get back an integer value that represents that character (basically). To create a character string you use double quotes ("string") and you get back an array of characters that is exactly as long as the string plus one more for the null character.

{ s, t, r, i, n, g, \0 }

Achithyn Wrote:I'm not sure, though would the following code actually work? Is 'board' a pointer? I just thought that you couldn't assign char's like that, less it was when the char was defined and initialized. So, something like userName = "Foo"; wouldn't work, right?

Quick warning: This part might be more advanced than you are ready for but since you asked...

Actually you are closer to the answer than you think... arrays are actually pointers!

You may have noticed that C will let you access elements 'outside' an array. I believe this is called a buffer overflow error. When you declare an array, space is allocated on your computer for exactly the size of the array, but since an array is a pointer, C has no way of knowing how large the array actually is. In fact, if you want you can do dangerous stuff like:

Code:

while(true)

{

  anArray++;

  printf("%s", anArray[0])

}

So my setupBoard function actually creates a new array and then uses our character array pointer to point to it. Actually I think %s with the characters might break... %s expects a null terminated string. %c is probably the one you should use if you are using characters. (Sorry about the sloppy code on my part, I haven't used C/C++ in almost a year! My current classes have used Java, Scheme, and Perl.)

Achithyn Wrote:I think your cat loop is very well thought out. However, it doesn't save too many lines of code... and (to me) can be kind of confusing to follow instead of what I had:
I can see how a loop, such as what you had, could work better in some cases though. I am sure that later on, as I grow more and more used to syntax and the use of it, the loop in your example would be more ideal.

Largely I wanted to put that there to show you how to loop through a 2d array safely as you will probably find other places where that is more useful.

Achithyn Wrote:Honestly, I don't really understand why my code works... can someone win by taking the last place on a tic tac toe board? Or are all victories and defeats determined before all places are taken up? If the last move was played in the last place available on the board, and that play would have constituted as a win, then would the program show: Winner! 'or' Cats! (?)

I think someone can win by taking the last place, but its very rare since a smart opponent will force you to block usually. Looking at your code, it appears that if the last piece was placed on the board and it won you should see "Winner!"; but you would then see "Cats!".

There are a probably a bunch of ways to prevent this (although if there is no such game it doesn't matter); the easiest one I see is to change the CheckWin function to return a boolean value of true if the player being tested won. Then in your main function do something like:

Code:

if( CheckWin(playerOne) )

  printf("You won!");

else

  CheckCat();

Whew, I think that's enough for now.