patterncppModerate
Ultimate Tic-Tac-Toe with AI and GUI
Viewed 0 times
withtoetictacultimateandgui
Problem
During my hiatus, I decided to revisit some of the Tic-Tac-Toe questions here. I created a simple Tic-Tac-Toe GUI game, and then thought, might as well do the Ultimate Tic-Tac-Toe challenge.
I am looking for ways to refactor my code. Particularly in
I think this is a mess. I spent very little time on this project and just started hacking away. Nonetheless, the computer does play a "perfect" game.
Here is the Github repo. I have uploaded a compiled binary for Linux.
MainWindow.h
```
#ifndef MAINWINDOW_H
#define MAINWINDOW_H
#include "dialog.h"
#include "tictactoe.h"
#include
#include
#include
#include
#include
#include
#include
#include
namespace Ui {
class MainWindow;
}
class MainWindow : public QMainWindow
{
Q_OBJECT
public:
int player = 1; //Keeps track of current
explicit MainWindow(QWidget *parent = 0);
void setUpGrid();
void setChar(char);
void Play();
void CheckWinner(int currentGrid);
void colorBoard(int nextGrid, int move);
void colorBoardWin(int nextGrid,int player);
void colorBoardUltimateWin(int player);
void computer(int grid, int player);
~MainWindow();
signals:
void turnComplete(int);
void computer_(int,int player);
public slots:
int itemClicked();
void humanMoves();
//void computer(int grid,int player);
void prediction(QString);
private slots:
void begin(int strength);
void invalidMove();
void computerMove(int,int);
void on_playAgain_clicked();
private:
Ui::MainWindow *ui;
Dialog *options = new Dialog;
QPushButton *itemButtons[9][5];
QGridLayout *layouts[9];
QFrame *frames[9];
QPushButton createButton(QString&, const QString/const char**/);
TicTacToe *game = new TicTacToe();
int strength;
bool humanTurn;
int currentGrid = -1;
int nextGrid = -1;
int wonGrids[9] = {};
int previousMove[2] = {-1,-1}; //Stores currentGrid, move
int test = -1;
QThread
I am looking for ways to refactor my code. Particularly in
MainWindow.cpp. I think this is a mess. I spent very little time on this project and just started hacking away. Nonetheless, the computer does play a "perfect" game.
Here is the Github repo. I have uploaded a compiled binary for Linux.
MainWindow.h
```
#ifndef MAINWINDOW_H
#define MAINWINDOW_H
#include "dialog.h"
#include "tictactoe.h"
#include
#include
#include
#include
#include
#include
#include
#include
namespace Ui {
class MainWindow;
}
class MainWindow : public QMainWindow
{
Q_OBJECT
public:
int player = 1; //Keeps track of current
explicit MainWindow(QWidget *parent = 0);
void setUpGrid();
void setChar(char);
void Play();
void CheckWinner(int currentGrid);
void colorBoard(int nextGrid, int move);
void colorBoardWin(int nextGrid,int player);
void colorBoardUltimateWin(int player);
void computer(int grid, int player);
~MainWindow();
signals:
void turnComplete(int);
void computer_(int,int player);
public slots:
int itemClicked();
void humanMoves();
//void computer(int grid,int player);
void prediction(QString);
private slots:
void begin(int strength);
void invalidMove();
void computerMove(int,int);
void on_playAgain_clicked();
private:
Ui::MainWindow *ui;
Dialog *options = new Dialog;
QPushButton *itemButtons[9][5];
QGridLayout *layouts[9];
QFrame *frames[9];
QPushButton createButton(QString&, const QString/const char**/);
TicTacToe *game = new TicTacToe();
int strength;
bool humanTurn;
int currentGrid = -1;
int nextGrid = -1;
int wonGrids[9] = {};
int previousMove[2] = {-1,-1}; //Stores currentGrid, move
int test = -1;
QThread
Solution
This will not be a full review of the code at all since I don't really review how it works, but here are some little things that may be of interest to make it better anyway :)
Qt5 new connection syntax
Qt5 introduces a new syntax for signals and slots which allows you to directly pass function pointers instead of relying on the Qt preprocessor to do the job. Your set of connections could be rewritten as:
There are many pros and some cons. You can look at the page I linked above for the details. Basically, the syntax is considered more compliated, but it gets typedefs and namespaces right since it does rely on the C++ type system and not on strings processing anymore. Moreover, the functions passed as slots don't even have to be slots anymmore; you can even pass lambdas as slots.
Use
Since you are using C++11, you should use
Don't use
While easy to use,
Qt5 new connection syntax
Qt5 introduces a new syntax for signals and slots which allows you to directly pass function pointers instead of relying on the Qt preprocessor to do the job. Your set of connections could be rewritten as:
connect(options, &Dialog::choosen,
this, MainWindow::begin);
connect(game, &TicTacToe::humanMoves,
this, &MainWindow::humanMoves);
connect(game, &TicTacToe::computerMove,
this, &MainWindow::computerMove);
connect(game, &TicTacToe::prediction,
this, &MainWindow::prediction);There are many pros and some cons. You can look at the page I linked above for the details. Basically, the syntax is considered more compliated, but it gets typedefs and namespaces right since it does rely on the C++ type system and not on strings processing anymore. Moreover, the functions passed as slots don't even have to be slots anymmore; you can even pass lambdas as slots.
Use
nullptrSince you are using C++11, you should use
nullptr instead of 0 and NULL to represent null pointers. It has the advantage of be being easy to search and to chose the pointer overload when a function is overloaded for integers and pointers:explicit MainWindow(QWidget *parent = nullptr);Don't use
std::randWhile easy to use,
std::rand is not the best tool to generate random numbers. For a simple game like this, it may not matter that much, but the C++11 header ` provides better alternatives to generate pseudo-random numbers. That's not critical for your application, but you should have a look at it.
Use the free functions std::begin and std::end
Once again it won't change anything for you, but since C++11, it is good practice to use the free functions std::begin and std::end instead of the container's methods. The reason if that they will also work with C-style arrays and std::valarray. This is a good habit to take, because it ensures that your code will still work if you change the container. Of course, the real advantage of these functions is that they make template code more generic.
std::fill(std::begin(boards),std::end(boards),std::vector(NUM_SQUARES,EMPTY));
begin is a half-reserved method name
While it is ok to name a method begin, a regular C++ user will expect that method to return an iterator and also will expect that another method named end, also returning an iterator, exists. There are no rules anywhere saying that this method name is reserved, but it is so commonly used for iteration that in practice it totally feels like a reserved name. To avoid a potential ambiguity, I would change the name begin to start or some equivalent in your class. That may help you or others to avoid some problems in the future.
Reorganize your includes
Whenever possible, incude the heades you need in the implementation file and not in the header. Also, try to remove the headers that are not used:
- Move
to tictactoe.cpp.
- Move
to MainWIndow.cpp.
- Remove
`, you don't need it.- I probably forgot some more headers that you could move/delete.
- Try to see whether you could use forward declarations to reduce the number of headers included in the .h file.
- C++11 in-class initializers are great, but initializing the variables in the constructors in the .cpp instead may allow you to only need forward declarations in the .h and to subsequently move of the includes to the .cpp files.
Code Snippets
connect(options, &Dialog::choosen,
this, MainWindow::begin);
connect(game, &TicTacToe::humanMoves,
this, &MainWindow::humanMoves);
connect(game, &TicTacToe::computerMove,
this, &MainWindow::computerMove);
connect(game, &TicTacToe::prediction,
this, &MainWindow::prediction);explicit MainWindow(QWidget *parent = nullptr);std::fill(std::begin(boards),std::end(boards),std::vector<int>(NUM_SQUARES,EMPTY));Context
StackExchange Code Review Q#68557, answer score: 11
Revisions (0)
No revisions yet.