Sign in to follow this  
sevak

Clean C# Code

Recommended Posts

sevak    100
Currently I am learning to program in C#. I recently created a Tic Tac Toe game but after reviewing my code, I could see that it was very messy. I was just wondering if anyone could give me a couple of tips on writing cleaner code? What have you guys found successful? Also can someone please give me the standards their company imposes on them. For example, does your company make you use Hungarian notation? Also, can you guys please give me some advice on bad programming habits.

Share this post


Link to post
Share on other sites
ITGER    163
You should post some code for a critique!

I'm not a big fan of Hungarian, and no one at my work uses it. C# and other modern languages have strong typing, so it's unnecessary and sometimes reduces readability.

Read code complete for more pointers, it's a very handy book.

Share this post


Link to post
Share on other sites
ldeej    308
For coding standards for C# a lot of people use the ".NET Framework Design Guidelines", to enforce those you can use "FXCop".

As far as programming habits, there are high level bad design decisions which could be improved by using "Design Patterns", and low level bad programming practices (i.e. not cleaning up your resources, using bad names for variables, bad names for functions, etc) are usually addressed by experience and advice.

If you post your code, or snippets of code areas that concern you then you might be able to get some advice about your design and your programming habits.

Google for any of the quoted terms and you will find more information if you are interested on those specific topics.

Share this post


Link to post
Share on other sites
sevak    100
Tic Tac Toe Game:

//Program.cs
using System;

namespace TicTacToe
{
class Program
{
static void Main(string[] args)
{
while (true)
{
TicTacToe.Display();
Console.Write("Play Another Round(y/n): ");
char ch = Convert.ToChar(Console.ReadLine());
if (Char.ToLower(ch) == 'y')
continue;
else
break;
}
}
}
}



//TicTacToe.cs
using System;

namespace TicTacToe
{
class TicTacToe
{
//Board Representation
static char[] achPositions = { '1', '2', '3', '4', '5', '6', '7', '8', '9' };
static public void Display()//Basic Board Interface
{
for(int i = 1; i <= 9;i++)
{
Console.WriteLine(" | | ");
Console.WriteLine(" {0} | {1} | {2} ", achPositions[0],
achPositions[1], achPositions[2]);
Console.WriteLine("-----|-----|-----");
Console.WriteLine(" | | ");
Console.WriteLine(" {0} | {1} | {2} ", achPositions[3],
achPositions[4], achPositions[5]);
Console.WriteLine("-----|-----|-----");
Console.WriteLine(" | | ");
Console.WriteLine(" {0} | {1} | {2} ", achPositions[6],
achPositions[7], achPositions[8]);
Console.WriteLine(" | | ");
if (Choice(i))
{
Console.WriteLine("You Win!");
break;
}
}
}
static bool Choice(int i)//Gets Input From Players
{
int iPositionChoice;
if (i % 2 != 0)
{
try
{
Console.Write("Player 1: ");
iPositionChoice = Convert.ToInt32(Console.ReadLine());
if (CheckChoice(1, iPositionChoice))
if (CheckWin())
return true;
else
return false;
else
throw new Exception();
}
catch
{
Console.WriteLine("Invalid Input.");
return Choice(i);
}
}
else
{
try
{
Console.Write("Player 2: ");
iPositionChoice = Convert.ToInt32(Console.ReadLine());
if (CheckChoice(2, iPositionChoice))
if (CheckWin())
return true;
else
return false;
else

throw new Exception();
}
catch
{
Console.WriteLine("Invalid Input.");
return Choice(i);
}
}
}
static bool CheckChoice(int iPlayer,int iChoice)//Checks the input
{//-1 is required to stop indexing out of range
if (Char.IsDigit(achPositions[iChoice - 1]))
{
achPositions[iChoice - 1] = (iPlayer == 1 ? 'X' : 'O');
return true;

}
else
return false;

}
static bool CheckWin()//Checks to see if player won the game
{
//Rows
if (achPositions[0] == achPositions[1] && achPositions[0] == achPositions[2])
return true;
else if (achPositions[3] == achPositions[4] && achPositions[3] == achPositions[5])
return true;
else if (achPositions[6] == achPositions[7] && achPositions[6] == achPositions[8])
return true;
//Collums
else if (achPositions[0] == achPositions[3] && achPositions[0] == achPositions[6])
return true;
else if (achPositions[1] == achPositions[4] && achPositions[1] == achPositions[7])
return true;
else if (achPositions[2] == achPositions[5] && achPositions[5] == achPositions[8])
return true;
//Diagonal
else if (achPositions[0] == achPositions[4] && achPositions[0]== achPositions[8])
return true;
else if (achPositions[2] == achPositions[4] && achPositions[2]== achPositions[6])
return true;
else
return false;
}

}
}

Share this post


Link to post
Share on other sites
jfclavette    1058
Quote:
Original post by sevak
Tic Tac Toe Game:
*** Source Snippet Removed ***
*** Source Snippet Removed ***


It's not bad at all. Of course, there are minor things I would change, but that's just me. Here are a few things worth lookging at, IMHO.

Converting:

while (true) {
TicTacToe.Display();
Console.Write("Play Another Round(y/n): ");
char ch = Convert.ToChar(Console.ReadLine());
if (Char.ToLower(ch) == 'y')
continue;
else
break;
}



do {
TicTacToe.Display();
Console.Write("Play Another Round(y/n): ");
char ch = Convert.ToChar(Console.ReadLine());
} while (Char.ToLower(ch) == 'y');



Makes it cleaner. Also, there are a few places where you could use some further error checking, but all in all, it looks nice to me for a learning experiment.

Share this post


Link to post
Share on other sites
chad_420    290
I think its alot neater to use brackets only on their own line and to use them even when they are implied like a oneliner if should look like this:

if(!no)
{
//...
}

but that kinda stuff is all personal preferences

Share this post


Link to post
Share on other sites
mutex    1111
You have some code duplication in TicTacToe.Choice. It can be rewritten like this:


static bool Choice(int i)//Gets Input From Players
{
if (i % 2 != 0)
{
Console.Write("Player 1: ");
}
else
{
Console.Write("Player 2: ");
}

try
{
int iPositionChoice = Convert.ToInt32(Console.ReadLine());
if (CheckChoice(i, iPositionChoice))
{
if (CheckWin())
return true;
else
return false;
}
else
{
throw new Exception();
}
}
catch
{
Console.WriteLine("Invalid Input.");
return Choice(i);
}
}




You also had an if statement without braces that contained another if statement. I'd add outer braces in such situations to make it clear where that last else belongs.

ldeej mentioned .NET Framework Design Guidelines, which is available here. I highly recommend reading it. Also remember that it's just a set of guidelines; don't freak out over trying to follow all the rules.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this