# Would like an opinion on my code.

## Recommended Posts

I have just created a TicTacToe game in win32 Console, using keyboard macro and gotoxy function, which both rely on Windows OS, anyways that is besides the point since I am not looking to porting it to another platform. I am mainly asking here how my coding structure, and coding habits look, I want to get some constructive critisism of my work, it is about 800 lines of coding, but that could be shortened if I wasn't so anal on my structure. For example,
for(int i = 0; i < whatever; i++) { /*do something...*/ };


in my coding I like to do...
for(int i = 0; i < whatever; i++)
{
//do something...
};


So to the point anyone have constructive critisism that may help me in the future please feel free to let me know. Below is a link to my executible file, source, and vs2008 project files. TicTacToe v1.0 [Edited by - wicked357 on February 13, 2010 4:53:07 AM]

##### Share on other sites

the only structures you would want to do that are

if(trueBool) small();

kind of things.

It is very important to be able to read code....

800 lines is not that long.

##### Share on other sites
I agree with AverageJoeSSU. One of the main things that I have been learning at school is that when you code you should optimize for readability.

I prefer:
for(int i = 0; i < whatever; i++){    //do something...}

almost infinitely to the other way of writing it that you have, just because since I have seen it that way so many times it makes it very easy for me to understand and interpret what is going on in the code.

There are some things in your code though that I think could be rewritten to improve reliability/readability:

You have:
OBJECT x1, x2, x3, x4, x5, x6, x7, x8, x9;OBJECT o1, o2, o3, o4, o5, o6, o7, o8, o9;

and then later when you're drawing the board:
//Draw X on game markers location	if((place.one == 1) && (x1.drawn == true))	{		drawX(x1.x, x1.y);	}	if((place.two == 1) && (x2.drawn == true))	{		drawX(x2.x, x2.y);	}	if((place.three == 1) && (x3.drawn == true))	{		drawX(x3.x, x3.y);	}	if((place.four == 1) && (x4.drawn == true))	{		drawX(x4.x, x4.y);	}	if((place.five == 1) && (x5.drawn == true))	{		drawX(x5.x, x5.y);	}	if((place.six == 1) && (x6.drawn == true))	{		drawX(x6.x, x6.y);	}	if((place.seven == 1) && (x7.drawn == true))	{		drawX(x7.x, x7.y);	}	if((place.eight == 1) && (x8.drawn == true))	{		drawX(x8.x, x8.y);	}	if((place.nine == 1) && (x9.drawn == true))	{		drawX(x9.x, x9.y);	}

I think you could use arrays to clean up this code and reduce copy-paste mistakes later, which I think would also make your code readable.
for(int i = 0; i < 9 /*Should be a const int for maintainability*/; i++){    if(board[i] == 1) //1 for X (could be changed to an enum {X, O, NOTHING})    {        drawX(xPieces[i].x, xPieces[i].y);    }    else if(board[i] == 2) //2 for O    {        drawO(oPieces[i].x, oPieces[i].y);       }}

This would be your entire code for drawing the x's and o's on the board instead of having the if chain you have.

If you use arrays like this through out your code you will be able to leverage loop structures to eliminate much of your duplicate code and make the code more readable.

Just a few thoughts.

## Create an account

Register a new account

• ### Forum Statistics

• Total Topics
628318
• Total Posts
2982043

• 9
• 9
• 13
• 11
• 15