patterncppMinor
Simple 8 Puzzle - ASCII
Viewed 0 times
simplepuzzleascii
Problem
I made a simple 8 puzzle which is 3x3 grids unsorted. The user has to move only one number at any direction, which is next to an empty tile. If the user sorted numbers, he/she wins the game, otherwise he/she needs to restart the game.
The game seems to work fine, but I would like to know how I can improve it further.
```
#include
#include
#include
#include
#include
#include
#include
#include
#define _USE_MATH_DEFINES
#include
#ifndef M_PI
#define M_PI 3.141592653589793238462643383f
#endif
namespace
{
enum KeyType
{
KUp = 72,
KRight = 77,
KLeft = 75,
KDown = 80
};
enum Direction
{
Up,
Down,
Left,
Right,
DirectionCount
};
std::mt19937 randomEngine()
{
std::array seed_data;
thread_local std::random_device source;
std::generate(std::begin(seed_data), std::end(seed_data), std::ref(source));
std::seed_seq seeds(std::begin(seed_data), std::end(seed_data));
thread_local std::mt19937 seeded_engine(seeds);
return seeded_engine;
}
float radian(Direction direction)
{
constexpr static std::array Radians
{
M_PI, // Up
0, // Down
-M_PI / 2, // Left
M_PI / 2 // Right
};
return Radians[direction];
}
}
struct Vector
{
Vector(unsigned int x, unsigned int y)
:x(x)
, y(y)
{}
unsigned int x, y;
};
class Puzzle
{
private:
constexpr static auto MaxTiles = 9u;
constexpr static auto Columns = 3u;
using Pair = std::pair;
using TileHolder = std::array;
public:
Puzzle()
{
std::iota(mTiles.begin(), mTiles.end(), '1');
mTiles.back() = ' ';
thread_local auto random = randomEngine();
std::shuffle(mTiles.begin(), mTiles.end(), random);
}
void update(Direction direction)
{
const auto& coord = getEmptyTileCoor
The game seems to work fine, but I would like to know how I can improve it further.
```
#include
#include
#include
#include
#include
#include
#include
#include
#define _USE_MATH_DEFINES
#include
#ifndef M_PI
#define M_PI 3.141592653589793238462643383f
#endif
namespace
{
enum KeyType
{
KUp = 72,
KRight = 77,
KLeft = 75,
KDown = 80
};
enum Direction
{
Up,
Down,
Left,
Right,
DirectionCount
};
std::mt19937 randomEngine()
{
std::array seed_data;
thread_local std::random_device source;
std::generate(std::begin(seed_data), std::end(seed_data), std::ref(source));
std::seed_seq seeds(std::begin(seed_data), std::end(seed_data));
thread_local std::mt19937 seeded_engine(seeds);
return seeded_engine;
}
float radian(Direction direction)
{
constexpr static std::array Radians
{
M_PI, // Up
0, // Down
-M_PI / 2, // Left
M_PI / 2 // Right
};
return Radians[direction];
}
}
struct Vector
{
Vector(unsigned int x, unsigned int y)
:x(x)
, y(y)
{}
unsigned int x, y;
};
class Puzzle
{
private:
constexpr static auto MaxTiles = 9u;
constexpr static auto Columns = 3u;
using Pair = std::pair;
using TileHolder = std::array;
public:
Puzzle()
{
std::iota(mTiles.begin(), mTiles.end(), '1');
mTiles.back() = ' ';
thread_local auto random = randomEngine();
std::shuffle(mTiles.begin(), mTiles.end(), random);
}
void update(Direction direction)
{
const auto& coord = getEmptyTileCoor
Solution
I'll start with update.
Since
For the direction offsets, you can just have arrays that store the x and y offsets for a particular direction rather than doing the math with floats.
That would allow you to get rid of your
draw
You're doing a lot of work in
Since
getEmptyTileCoordinates() returns a Pair, you can store that in coord instead of using a reference:auto coord(getEmptyTileCoordinates());For the direction offsets, you can just have arrays that store the x and y offsets for a particular direction rather than doing the math with floats.
// in local namespace
const int x_offset[] = {0, 0, -1, +1}; // Up, Down, Left, Right
const int y_offset[] = {-1, +1, 0, 0};
// in update
unsigned x = coord.first.x + x_offset[direction];
unsigned y = coord.first.y + y_offset[direction];That would allow you to get rid of your
radian function. And since x and y are unsigned, they'll never be less than zero and you don't need to check them for that (although the compiler will know this and likely will optimize away those comparisons).draw
You're doing a lot of work in
draw to determine when to use a newline instead of a space. This can be simplified by using a counter.void draw(std::ostream& stream) const
{
int column = 0;
for (const auto& tile : mTiles)
{
column = (column + 1) % Columns;
stream << tile << (column == 0 ? '\n' : ' ');
}
}Code Snippets
auto coord(getEmptyTileCoordinates());// in local namespace
const int x_offset[] = {0, 0, -1, +1}; // Up, Down, Left, Right
const int y_offset[] = {-1, +1, 0, 0};
// in update
unsigned x = coord.first.x + x_offset[direction];
unsigned y = coord.first.y + y_offset[direction];void draw(std::ostream& stream) const
{
int column = 0;
for (const auto& tile : mTiles)
{
column = (column + 1) % Columns;
stream << tile << (column == 0 ? '\n' : ' ');
}
}Context
StackExchange Code Review Q#117440, answer score: 2
Revisions (0)
No revisions yet.