Very basic Tic-Tac-Toe game

Question

This is working C++ code for Tic-Tac-Toe (even not using arrays):

#include "stdafx.h"
#include <iostream>
using namespace std;
using namespace System;


int Decider(char a, char b, char c, char d, char e, char f, char g, char h, char i, int p);
int Win(char a, char b, char c, char d, char e, char f, char g, char h, char i);
int Loop(char a, char b, char c, char d, char e, char f, char g, char h, char i, int p);
int WinCheck(char a, char b, char c, char d, char e, char f, char g, char h, char i, int p, char z);

void Board(char a, char b, char c, char d, char e, char f, char g, char h, char i)

{
system("cls");

cout << "\n\t    Tic-Tac-Toe\n\n\n";

cout <<
    "\t+-----+-----+-----+\n"
    "\t|     |     |     |\n"
    "\t|  " << a << "  |  " << b << "  |  " << c << "  |\n"
    "\t|     |     |     |\n"
    "\t+-----+-----+-----+\n"
    "\t|     |     |     |\n"
    "\t|  " << d << "  |  " << e << "  |  " << f << "  |\n"
    "\t|     |     |     |\n"
    "\t+-----+-----+-----+\n"
    "\t|     |     |     |\n"
    "\t|  " << g << "  |  " << h << "  |  " << i << "  |\n"
    "\t|     |     |     |\n"
    "\t+-----+-----+-----+\n\n\n";

}

void Layout()
{
Board('7', '8', '9', '4', '5', '6', '1', '2', '3');
cout << "\n\t This is the layout"
    "\n\n\n\tUse your number pad " 
    "\n\t       without"
    "\n      worrying about numbers!"
    "\n\n\n    Enter anything to start! : ";
int x;
cin >> x;
}


int Decider(char a, char b, char c, char d, char e, char f, char g, char h, char i, int p)
{
int P = p % 2;
char Z;
Z = (P == 1) ? 'X' : 'O';

cout << "Player "<<Z<<"'s Choice: ";
char q;
cin >> q;

if (q == '7' && a != 'X' && a != 'O')
    a = Z;

else if (q == '8' && b != 'X' && b != 'O')
    b = Z;

else if (q == '9' && c != 'X' && c != 'O')
    c = Z;

else if (q == '4' && d != 'X' && d != 'O')
    d = Z;

else if (q == '5' && e != 'X' && e != 'O')
    e = Z;

else if (q == '6' && f != 'X' && f != 'O')
    f = Z;

else if (q == '1' && g != 'X' && g != 'O')
    g = Z;

else if (q == '2' && h != 'X' && h != 'O')
    h = Z;

else if (q == '3' && i != 'X' && i != 'O')
    i = Z;


else
{
    cout << "\n\nInvalid Move!\n\n";
    Decider(a, b, c, d, e, f, g, h, i, p);
}

p = p + 1;

WinCheck(a, b, c, d, e, f, g, h, i, p, Z);

return 0;
}
int WinCheck(char a, char b, char c, char d, char e, char f, char g, char h, char i, int p, char z)
{
int X = Win(a, b, c, d, e, f, g, h, i); 
if (X == 0)
    Loop(a, b, c, d, e, f, g, h, i, p);
else if (X == 1)
{
    Board(a, b, c, d, e, f, g, h, i);
    cout << "\t  Player '" << z << "' wins!\n\n\n\n";
}
else
{
    Board(a, b, c, d, e, f, g, h, i);
    cout << "\t  Match is Draw!\n\n\n\n";
    return 0;

}
}

int Win(char a, char b, char c, char d, char e, char f, char g, char h, char i)
{
if (a == b && b == c && b != ' ')
    return 1;
else if (d == e && e == f && e != ' ')
    return 1;
else if (g == h && h == i && h != ' ')
    return 1;

else if (a == e && e == i && e != ' ')
    return 1;
else if (c == e && e == g && e != ' ')
    return 1;

else if (a == d && d == g && d != ' ')
    return 1;
else if (b == e && e == h && e != ' ')
    return 1;
else if (c == f && f == i && f != ' ')
    return 1;

else if (a != ' ' && b != ' ' && c != ' ' &&
    d != ' ' && e != ' ' && f != ' ' &&
    g != ' ' && h != ' ' && i != ' ')
    return 2;

else
    return 0;
}

int Loop(char a, char b, char c, char d, char e, char f, char g, char h, char i, int p)
{
Board(a, b, c, d, e, f, g, h, i);
Decider(a, b, c, d, e, f, g, h, i, p);

return 0;
}

int main()
{
Console::SetWindowSize(36, 40);

Layout();

char a = ' '; char b = ' '; char c = ' ';
char d = ' '; char e = ' '; char f = ' ';
char g = ' '; char h = ' '; char i = ' ';

int p = 1;

Loop(a, b, c, d, e, f, g, h, i, p);

return 0;
}

Is there any way to improve this?

Answer 1

Where to start?

Using Namespace!

Never do this.

using namespace std;
using namespace System;

This pollutes the namespace and will cause all sorts of problems in anything larger than a toy program. The problem with using it in toy programs is that it becomes a habit that will lead you to accidentally use it in real code. Stop this immediately do not form this habit.

If you don't understand the problem with polluing namespaces read the question: Why is “using namespace std;” considered bad practice?

Layout

Some indentation would be nice. It is normal to indent all code between {} (though there is a style that does not do this it is rare). Also you need to be consistent about your indenting. The style of indenting should ne consistent throughout your code.

Sub Statements

Sub statements should always be inside {}. Even though some statements allow sub statements if (<cond>) <sub-statement> or for(<test>) <sub-statement>. It is not always obvious that the following statement is a single statement.

if (x != 2)
    boilTheEggs(x);

Is that code doing what you expect? Depends. What is boilTheEggs? If it is a function call fine. But it could be some cursed badly written macro that expands into multiple statements. It is best to always use '{}` to protect yourself (and your maintainers) from changes.

 if (x != 2) {
     boilTheEggs(x);
 }

Will always work as expected.

Parameter Names

int Decider(char a, char b, char c, char d, char e, char f, char g, char h, char i, int p);

Sure: But I have no idea what any of those parameters are being used for. Nor will you in 6 months time. Make the code easy to read. When you come back in 6 months you will thank yourself.

Don't hard code platfrom specific code.

system("cls");

This may clear the screen on windows. But generates an ugly message on any other system. Use some macros to detect what OS you are on.

clearScreen();

Then in your header file:

#if defined(__WINDOWS__)
void clearScreen() {
     system("cls");
}
#elif defined(__MY_FNACY_OS__)
void clearScreen() {
     blue();
}
#else
void clearScreen() {
     // Don't know how
     // but not important enough to stop the app.
}
#endif

Users are stupid or malicious

Always validate user input. They will provide the most awful input they can. You should validate any data coming from a living human.

int x;
cin >> x;

If I type in X<Enter> Then it will completely lock the system as the stream std::cin will have its bad bit set. Any further attempts to read from it will fail until the bad bit is reset.

Identifier Names.

Normally in C++:

• user defined type identifiers begin with an "UpperCase" letter.
• object identifers begin with a lower case letter.

Types are the most important part of C++. By using this convention we can easily spot types in the code and separate them from objects. (Note function/methods are also objects as you can take their address).

While we are on the subject. Like parameter names; all variable names should be self documenting. This means use a self descriptive names so that when you use it in an equation you can understand what is going on.

int P = p % 2;
char Z;
Z = (P == 1) ? 'X' : 'O';

No idea what is happening here as the objects have random gibberish one character names that do not portray their meaning.

Some developers prefer variables to have noun like identifiers while function/methods have verb like identifiers. Its a reasonable base rule but I follow it to the extent that it makes my code readable. Not I usually use multi word names to make the code usable.

int  oddPlayer   = player % 2;
char playerToken = (oddPlayer) ? 'X' : 'O';

Answer 2

Couple code layout changes would improve readability of the code. For example indentation of function bodies and empty lines in correct places:

For example this part of the code:

WinCheck(a, b, c, d, e, f, g, h, i, p, Z);

return 0;
}
int WinCheck(char a, char b, char c, char d, char e, char f, char g, char h, char i, int p, char z)
{
int X = Win(a, b, c, d, e, f, g, h, i); 
if (X == 0)
    Loop(a, b, c, d, e, f, g, h, i, p);

Could be:

    WinCheck(a, b, c, d, e, f, g, h, i, p, Z);

    return 0;
}

int WinCheck(char a, char b, char c, char d, char e, char f, char g, char h, char i, int p, char z)
{
    int X = Win(a, b, c, d, e, f, g, h, i); 
    if (X == 0)
        Loop(a, b, c, d, e, f, g, h, i, p);

You could also use an object for storing state of board instead of 9 chars variables.

Anyway, Fresh idea to use a recursion instead of a loop to solve the problem.

Answer 3

You could have used switch case instead of typing if again and again.