patterncppMinor
Text-based Tetris game - follow-up
Viewed 0 times
textgamefollowbasedtetris
Problem
Previous question:
Text-based Tetris game
I don't have a Linux terminal to be sure whether or not I have implemented the Linux version correctly, but hopefully it's OK.
Summary of improvements:
I would like to know if there is anything I should take care of before processing the code.
```
#include
#include
#include
#include
#ifdef __linux__
/***
kbhit() and getch() for Linux/UNIX
Chris Giese http://my.execpc.com/~geezer
Release date: ?
This code is public domain (no copyright).
You can do whatever you want with it.
/
#include / struct timeval, select() /
/ ICANON, ECHO, TCSANOW, struct termios /
#include / tcgetattr(), tcsetattr() /
#include / atexit(), exit() /
#include / read() /
#include / printf() /
static struct termios g_old_kbd_mode;
/***
***/
static void cooked(void)
{
tcsetattr(0, TCSANOW, &g_old_kbd_mode);
}
/***
***/
static void raw(void)
{
static char init;
/**/
struct termios new_kbd_mode;
if (init)
return;
/ put keyboard (stdin, actually) in raw, unbuffered mode /
tcgetattr(0, &g_old_kbd_mode);
memcpy(&new_kbd_mode, &g_old_kbd_mode, sizeof(struct termios));
new_kbd_mode.c_lflag &= ~(ICANON | ECHO);
new_kbd_mode.c_cc[VTIME] = 0;
new_kbd_mode.c_cc[VMIN] = 1;
tcsetattr(0, TCSANOW, &new_kbd_mode);
/ when we exit, go back to normal, "cooked" mode /
Text-based Tetris game
I don't have a Linux terminal to be sure whether or not I have implemented the Linux version correctly, but hopefully it's OK.
Summary of improvements:
- More OOP
- Improved the names of the classes and their functions and I hope they're OK now
- More portable, isolated platform-specific code
- Eliminated "magic numbers"
I would like to know if there is anything I should take care of before processing the code.
```
#include
#include
#include
#include
#ifdef __linux__
/***
kbhit() and getch() for Linux/UNIX
Chris Giese http://my.execpc.com/~geezer
Release date: ?
This code is public domain (no copyright).
You can do whatever you want with it.
/
#include / struct timeval, select() /
/ ICANON, ECHO, TCSANOW, struct termios /
#include / tcgetattr(), tcsetattr() /
#include / atexit(), exit() /
#include / read() /
#include / printf() /
static struct termios g_old_kbd_mode;
/***
***/
static void cooked(void)
{
tcsetattr(0, TCSANOW, &g_old_kbd_mode);
}
/***
***/
static void raw(void)
{
static char init;
/**/
struct termios new_kbd_mode;
if (init)
return;
/ put keyboard (stdin, actually) in raw, unbuffered mode /
tcgetattr(0, &g_old_kbd_mode);
memcpy(&new_kbd_mode, &g_old_kbd_mode, sizeof(struct termios));
new_kbd_mode.c_lflag &= ~(ICANON | ECHO);
new_kbd_mode.c_cc[VTIME] = 0;
new_kbd_mode.c_cc[VMIN] = 1;
tcsetattr(0, TCSANOW, &new_kbd_mode);
/ when we exit, go back to normal, "cooked" mode /
Solution
Just a few quick comments on things I saw when scanning through:
By defining this as a
There still seem to be a lot of magic numbers in the code. Some of them are repeated.
could be something like
I don't understand when the following triggers:
It's probably in the code somewhere, but I don't feel like tracking it down now. A comment of why we are comparing to
struct Block : public NonCopyableBy defining this as a
struct, you make members public (unless overridden). As a general rule, it is preferred for members to be private as a default. If you are marking a member as public then you should have a specific reason for that member. That pushes you to write code that does not rely on the members being public. You do not seem to be publicly using your public members, so you might as well make this a class. Note: this is not to say that there is never a reason to use a struct or public member, just that I see no sign of such a reason here. block.resize(4, std::vector(4, 0));
field.resize(22, std::vector(13, 0));
stage.resize(22, std::vector(13, 0));
int x = 4;
Random getRandom{ 0, 6 };
for (size_t i = 0; i <= board.size() - 2; ++i)
for (size_t j = 0; j <= board.rowSize() - 2; ++j)
if ((j == 0) || (j == 11) || (i == 20))
board.getDim(i, j) = stage.getDim(i, j) = 9;
x = 4;
COORD coord = { 20, 20 };There still seem to be a lot of magic numbers in the code. Some of them are repeated.
Random getRandom{ 0, 6 };could be something like
Random getRandom{ 0, blocklist.size() };I don't understand when the following triggers:
if (board.getDim(i, j + block.size()) > 1)
{
gameOver = true;It's probably in the code somewhere, but I don't feel like tracking it down now. A comment of why we are comparing to
1 would be helpful.Code Snippets
struct Block : public NonCopyableblock.resize(4, std::vector<int>(4, 0));
field.resize(22, std::vector<int>(13, 0));
stage.resize(22, std::vector<int>(13, 0));
int x = 4;
Random getRandom{ 0, 6 };
for (size_t i = 0; i <= board.size() - 2; ++i)
for (size_t j = 0; j <= board.rowSize() - 2; ++j)
if ((j == 0) || (j == 11) || (i == 20))
board.getDim(i, j) = stage.getDim(i, j) = 9;
x = 4;
COORD coord = { 20, 20 };Random getRandom{ 0, 6 };Random getRandom{ 0, blocklist.size() };if (board.getDim(i, j + block.size()) > 1)
{
gameOver = true;Context
StackExchange Code Review Q#74357, answer score: 3
Revisions (0)
No revisions yet.