Что ж, Пакка, как и просил - держи рецензию на свой код
Прокомментирую его так, будто это настоящий продакшн-код:
PHP Code:
//---------------------------------------------------------------------------
#include <vcl.h>
#include <math.h>
#pragma hdrstop
#include "Unit1.h"
#include <vector>
//---------------------------------------------------------------------------
#pragma package(smart_init)
#pragma resource "*.dfm"
using namespace std;
TForm1 *Form1;
int mainX, mainY, raznica_ammo_enemy_Y, ammoN, n;
bool build;
const int enemyN=5;
class Tower
{
public:
int xx;
int yy;
int health;
int damage;
int target;
int reload;
Tower ( int x, int y, int h, int d, int t, int r);
void Draw_Tower();
void Zahvat_Target();
void Draw_Lazer();
void Reload_Tower();
};
class Enemy
{
public:
int xx;
int yy;
int health;
Enemy( int x, int y, int h );
void Draw_Enemy();
void Move_Enemy();
};
class Ammo
{
public:
int xx;
int yy;
int number;
int target;
Ammo (int x, int y, int n, int t);
void Move_Ammo( int j);
void Spawn_Ammo( int j);
void Draw_Ammo();
};
vector <Enemy> enemy_group;
vector <Tower> tower_group;
vector <Ammo> ammo_group;
vector <int> interval;
Graphics::TBitmap *enemy2 ;
Graphics::TBitmap *ammo ;
Graphics::TBitmap *tower ;
Graphics::TBitmap *boom ;
Graphics::TBitmap *main ;
//---------------------------------------------------------------------------
__fastcall TForm1::TForm1(TComponent* Owner)
: TForm(Owner)
{
}
//---------------------------------------------------------------------------
Enemy::Enemy( int x, int y, int h )
{
xx=x;
yy=y;
health=h;
}
//----------------------------
void Enemy::Draw_Enemy()
{
for (int i=0; i<enemyN; i++)
{
if (enemy_group[i].xx!=mainX)
{
if (enemy_group[i].health>0)
{
Form1->Canvas->Draw(enemy_group[i].xx, enemy_group[i].yy, enemy2);
}
else
{
Form1->Canvas->Draw(enemy_group[i].xx, enemy_group[i].yy, boom);
}
}
}
}
//---------------------
void Enemy::Move_Enemy()
{
if ( xx != mainX )
{
if (xx>0)
{
xx+=1;
}
}
}
//===========================
Tower::Tower( int x, int y, int h, int d, int t, int r)
{
xx=x;
yy=y;
health=h;
damage=d;
target=t;
reload=r;
}
//---------------
void Tower::Zahvat_Target()
{
int rasstoyanie[5]; //Массив расстояний от башни до врага
for (int j=0; j < enemyN; j++)
{
if (enemy_group[j].health>0)
{
int raznicaXtarget = xx - enemy_group[j].xx; //Разница по Х
int raznicaYtarget = yy - enemy_group[j].yy; //Разница по Y
if (raznicaYtarget<0){ //Если башня выше цели
raznicaYtarget=enemy_group[j].yy - yy;
}
rasstoyanie[j]=sqrt(pow(raznicaXtarget,2)+pow(raznicaYtarget,2)); //Расстояние от врага до башни, по Пифагору
}
int min = rasstoyanie[0];
int b=0;
for (int j=0; j<enemyN; j++){
if (rasstoyanie[j]<min){
min=rasstoyanie[j]; //Находим ближайшего врага
b=j;
}
if (rasstoyanie[j]<350)
{ //Если враг в области поражения башни (пока числом)
target=b; //Запоминаем номер врага
}
}
}
}
//-------------
void Tower::Draw_Tower()
{
Form1->Canvas->Draw(xx, yy, tower);
}
//-------------
void Tower::Draw_Lazer()
{
Form1->Canvas->Pen->Color=(TColor)RGB(200,0,0);
Form1->Canvas->MoveTo(xx,yy);
Form1->Canvas->LineTo(enemy_group[target].xx,enemy_group[target].yy);
}
//--------------
void Tower::Reload_Tower()
{
reload+=1;
}
//=====================================================================
Ammo::Ammo (int x, int y, int n, int t)
{
xx=x;
yy=y;
number=n;
target=t;
}
//-----------------------
void Ammo::Move_Ammo( int j )
{
for(int j=0; j<ammoN; j++) // Для каждого патрона из массива... Делать столько раз, сколько патронов в потоке.
{
if ( enemy_group [target].xx == xx && enemy_group[target].yy == yy )
{
enemy_group[target].health-=50;
}
else
{
xx-=1; // Движение по оси Х
//Разница между высотой врага и патрона=высота патрона - высота"выделенного" врага
raznica_ammo_enemy_Y=0;
if (enemy_group[target].yy > yy) //Если высота врага < высота патрона
{
raznica_ammo_enemy_Y = enemy_group[target].yy - yy;
if (raznica_ammo_enemy_Y < 5)
{
yy += raznica_ammo_enemy_Y;
}
else
{
yy += 5; //Опускаемся на 5
}
}
else
{
raznica_ammo_enemy_Y = yy - enemy_group[target].yy;
if (raznica_ammo_enemy_Y<5)
{
yy -= raznica_ammo_enemy_Y;
}
else
{
yy -= 5; //Поднимаемся на 5
}
}
}
}
}
//--------------------------
void Ammo::Spawn_Ammo ( int j )
{
if ( tower_group[j].target >= 0 && tower_group[j].target < 5 ) //Если у башни есть цель
{
if (enemy_group[tower_group[j].target].health>0)
{
Ammo newAmmo (tower_group[j].xx, tower_group[j].yy, ammoN, tower_group[j].target);
ammo_group.push_back( newAmmo );
ammoN++;
tower_group[j].reload=0; //Обнуляем для нового отсчета
}
}
}
//-----------------
void Ammo::Draw_Ammo()
{
Form1->Canvas->Draw( xx, yy, ammo);
}
//===========================
void __fastcall TForm1::Button1Click(TObject *Sender)
{
for (int i=0; i < enemyN; i++)
{
Enemy newEnemy( random(100),random(300), 100 );
enemy_group.push_back( newEnemy );
}
Timer1->Enabled = true;
}
//---------------------------------------------------------------------------
void __fastcall TForm1::Timer1Timer(TObject *Sender)
{
for (int i=0; i<enemyN; i++)
{
enemy_group[i].Move_Enemy();
}
//
if (tower_group.empty()!=true)
{
for (int i=0; i<tower_group.size(); i++)
{
tower_group[i].Zahvat_Target(); //Башни ищут ближайшую цель
}
for(int j=0; j<n; j++)
{
if (tower_group[j].reload==10)
{
ammo_group[j].Spawn_Ammo( j );
}
else
{
tower_group[j].Reload_Tower();
}
}
for (int k=0; k<ammoN; k++)
{
ammo_group[k].Move_Ammo( k ); //Двигаем все патроны
}
}
Invalidate();
}
//---------------------------------------------------------------------------
void __fastcall TForm1::FormPaint(TObject *Sender)
{
for (int i=0; i<enemyN; i++)
{
enemy_group[i].Draw_Enemy();
}
if (tower_group.empty()!=true)
{
for (int i=0; i<tower_group.size(); i++)
{
tower_group[i].Draw_Tower();
if ( tower_group[i].target >= 0 ) //Если у башни есть цель то рисуем "лазер"
{
if (enemy_group[tower_group[i].target].health>0)
{
tower_group[i].Draw_Lazer();
}
}
}
for(int j=0; j < ammoN; j++)
{
ammo_group[j].Draw_Ammo();
}
}
Form1->Canvas->Draw(610, 235, main);
}
//---------------------------------------------------------------------------
void __fastcall TForm1::FormCreate(TObject *Sender)
{
DoubleBuffered=true;
for (int i=0; i < enemyN; i++)
{
Enemy newEnemy( random(100),random(300), 100 );
enemy_group.push_back( newEnemy );
}
n=0;
mainX=610;
mainY=235;
ammoN=0;
tower = new Graphics::TBitmap;
tower->LoadFromFile("tower.bmp");
main = new Graphics::TBitmap;
main->LoadFromFile("main.bmp");
enemy2 = new Graphics::TBitmap;
enemy2->LoadFromFile("enemy2.bmp");
ammo = new Graphics::TBitmap;
ammo->LoadFromFile("ammo.bmp");
boom = new Graphics::TBitmap;
boom->LoadFromFile("boom.bmp");
}
//---------------------------------------------------------------------------
void __fastcall TForm1::FormMouseDown(TObject *Sender, TMouseButton Button,
TShiftState Shift, int X, int Y)
{
if (build==true)
{
Tower newTower ( X,Y, 100, 50, -1, 10);
tower_group.push_back( newTower );
n++;
}
Invalidate();
Form1->Edit1->Text=tower_group.size();
}
//---------------------------------------------------------------------------
void __fastcall TForm1::Button2Click(TObject *Sender)
{
if (build==true){
build=false;
}
else {build=true;}
}
//---------------------------------------------------------------------------
Оформление:
1) Бросается в глаза разный стиль именования переменных:
mainX, mainY, raznica_ammo_enemy_Y. Нотация везде должна быть единой, даже если эта нотация - венгерская.
2) Конструктор класса
Tower вынесен за его пределы - это ладно
(хотя и не ясно, зачем), но почему он записан где-то в середине файла, а не сразу вслед за классом? А ещё лучше - выносить классы в отдельные файлы.
3) Да и что значат его переменные, почему они все однобуквенны? В строке
Tower ( int x, int y, int h, int d, int t, int r) разобраться сможет только автор этого кода, и никто другой. А писать код надо так, будто
сопровождать его будет склонный к насилию психопат - который знает, где ты живёшь. (с)
4) За названия вроде
"raznica_ammo_enemy_Y" или
"Zahvat_Target()" нужно вырвать и застрелить руки. Если уж не знаешь полностью английского
(что вполне простительно) - то пиши всё транслитом, но смешивать языки - это моветон.
5) Все символы, относящие к названию типа, нужно писать слитно. В строке
vector <Enemy> у тебя
Enemy является частью типа, поэтому пробел ставить не надо. Это же касается и звёздочек в объявлениях указателей.
6) А вот в строках типа
enemy_group[i].xx!=mainX, наоборот, нужно обязательно отделять пробелом знаки операций. Иначе визуально ничего не понятно. Арифметические операции - это полноценные функции, и они заслуживают "отделения".
7) Скобки
{} кое-где поплыли, а кое-где и отступы вместе с ними. Местами вообще получилась нечитабельная каша, как в функции
Zahvat_Target().
Кодинг:
1) Строка
#include <vector> должна стоять выше, чем
#pragma hdrstop. Потому как в твоём случае <vector> будет инклудиться всегда, хотя достаточно всего одного раза.
2) Используется множество глобальных переменных - которые, как известно, не есть гут. В большинстве случаев их можно заменить локальными, если только это не синглтоны. Да и хотя бы прокомментировать их не мешало бы.
3) Из-за конструкций вроде
Form1->Canvas->Draw(610, 235, main) есть риск быть похороненным в закрытом гробу бирюзового цвета.
Почему именно бирюзового? Потому что 610. А если уж эта строка нужна чисто "для дебага", то нужен комментарий об этом
(а ещё лучше - условная компиляция).
4) В конструкторах лучше использовать списки инициализации, а не поочерёдное присваивание. Во-первых, так безопаснее. Во-вторых, только так можно проинициализировать не-статические константы класса.
5) Кстати, а где константы-то? В коде хватает переменных, которые не изменяются после инициализации. А ведь "константность" защищает от ошибок на этапе исполнения.
6) Раздражают конструкции вида
if (tower_group.empty()!=true) или
reload+=1. В первом случае сравнение с
true можно не писать, а во втором нужен обычный инкремент.
7) Приведение типов в стиле C, как в строке
(TColor)RGB(200,0,0), не всегда безопасно - лучше использовать "плюсовые"
static_cast и
dynamic_cast.
8) Зачем в функцию
Move_Ammo( int j ) передаётся параметр
j, когда он же вводится в первой строчке самой функции? Либо неиспользуемая переменная, либо просто ошибка.
9) Ввело в ступор содержимое функции
Button2Click(). Надеюсь, что это просто "заглушка" такая?
Рекомендации:
1) Использовать везде единый стиль записи. Это касается именования переменных, расстановки скобок и комментариев. Имена должны быть осмысленными.
2) Упаковывать данные внутрь классов, используя
идиому RAII. Если картинка относится к элементу конкретного класса, не надо таскать её как глобальную переменную - нужно выделить память под неё в конструкторе, и освободить в деструкторе.
3) Почему не стоит обходить вектор через int-переменную, скромно умолчу. Но на всякий случай посоветую почитать про итераторы. Через них всё идёт намного быстрее.
4) Вместо "голых" указателей лучше использовать
shared_ptr<> и создавать всё в стеке, избегая кучи вообще. Хотя тебе ещё далековато до такой техники.
5) Вместо множества функций типа
Draw_Tower() и
Draw_Lazer() лучше иметь одну перегруженную функцию
Draw(), которая могла бы принимать объекты разных типов. Это удобно для вызова функций
(IDE сама показывает перегрузки), да и наследоваться от таких классов куда проще.
6) В любом классе желательно наличие конструктора по умолчанию
(без параметров), дабы его объекты можно было использовать внутри массивов и STL-контейнеров. У тебя же далеко не везде так.
P.S. Только не стоит расстраиваться от прочитанного - я в твоём возрасте кодил намного хуже
Search
Member List
Help
