Вопрос или проблема
Я столкнулся с некоторым кодом, который заставил меня задуматься, правильно ли сделаны вещи, избегая не определенного поведения и других ловушек. Ниже представлена упрощенная версия дизайна. Представьте себе игру, в которой игрок с определенным уровнем мастерства стреляет из ружья. Ружье наносит некоторый базовый урон противнику, и урон усиливается мастерством игрока. Кроме того, если мастерство игрока падает ниже некоторого минимального значения, отдача ружья наносит урон самому игроку, уменьшая его здоровье.
В коде есть два класса: Player
и Rifle
. Объект Player
имеет количество очков здоровья, некоторый уровень мастерства и ружье. Объект Rifle
имеет свой базовый урон для противников, а также базовый урон, наносимый отдачей. Объект Rifle
также хранит указатель на объект Player
, которому он принадлежит. Код выглядит так:
// example.cpp
#include <iostream>
struct Player;
struct Rifle {
Rifle(Player *p) : player{p} {}
int shoot();
int damage{8};
int recoil{1};
Player *player{};
};
struct Player {
Player(const int hp, const int s) : health_points{hp}, skill{s} {}
int shoot();
Rifle rifle{this};
int health_points{};
int skill{};
bool dead{false};
};
int Rifle::shoot() {
if (player->skill < 5) {
player->health_points -= recoil;
}
return player->skill * damage;
}
int Player::shoot() {
int damage{rifle.shoot()};
if (health_points < 1) {
dead = true;
}
return damage;
}
int main() {
Player player{100, 3};
std::cout << "Игрок HP: " << player.health_points << "\n";
player.shoot();
player.shoot();
player.shoot();
std::cout << "Игрок HP: " << player.health_points << "\n";
}
Компиляция с помощью clang++ example.cpp -std=c++20
дает
Игрок HP: 100
Игрок HP: 97
Не очень искусный снайпер!
Вопросы по члену Player*
в Rifle
:
- Является ли это “хорошим” дизайном, или мы можем инкапсулировать эффекты на
Player
после использования егоRifle
более разумным образом? - Безопасен ли этот подход с точки зрения памяти?
- Если классы значительно больше по размеру, и этот дизайн имеет смысл, как можно использовать кучу/умные указатели, если это применимо?
- Если есть какая-либо другая дополнительная критика, которую я не учитываю, пожалуйста, сообщите об этом!
Ответ или решение
Изучив описанный вами код, давайте рассмотрим основные аспекты проектирования, обеспечения безопасности и управления памятью в контексте классов Player
и Rifle
, а также ответим на заданные вопросы.
1. Оценка дизайна
Данный дизайн имеет несколько недостатков с точки зрения принципов инкапсуляции и разделения ответственности. Класс Rifle
знает о Player
, но это создает жесткую связь между двумя классами.
С точки зрения объектно-ориентированного программирования (ООП), если «ружье» вызывает эффект на «игрока», то это указывает на то, что два класса должны быть более независимыми:
-
Используйте события или делегаты: Вместо того чтобы иметь указатель
Player *p
внутри классаRifle
, можно создать механизм событий, который будет оповещатьPlayer
о том, что он должен обновить свои параметры (например, здоровье) после использования ружья. -
Рефакторинг методов: Правильнее было бы перенести логику расчета урона и влияния на здоровье непосредственно в метод
Player::shoot()
, оставивRifle
только для определения базовых параметров, таких как урон и отдача.
2. Безопасность памяти
В данной реализации есть несколько проблем, связанных с безопасностью памяти:
-
Указатель на
Player
: Использование обычного указателяPlayer* player
создает риск утечки памяти или аварийного завершения программы, если объектPlayer
будет уничтожен до того, как будет вызван метод классаRifle
. -
Нет проверки на
nullptr
: В методеRifle::shoot()
отсутствует проверка на нулевой указательplayer
. Если по какой-то причине указатель окажется нулевым, программа завершится с аварией.
Для повышения безопасности можно использовать удобные умные указатели (std::shared_ptr
или std::unique_ptr
), которые автоматически управляют временем жизни объектов.
3. Использование кучи и "умных указателей"
Если классы имеют большую размерность и сложность, использование умных указателей будет уместным:
-
std::shared_ptr
: ЕслиRifle
может иметь несколько владельцев (например, если несколько объектов обращаются к одной и той жеPlayer
), то можно использоватьstd::shared_ptr<Player>
. Это позволяет предотвратить ручное управление памятью и автоматизировать удаление объекта при отсутствии на него ссылок. -
std::unique_ptr
: Если классRifle
является единственным владельцем объектаPlayer
, разумно использоватьstd::unique_ptr<Player>
, что также обеспечит автоматическое освобождение памяти.
4. Дополнительная критика
-
Управление состоянием: Необходимо более аккуратно управлять состоянием
Player
иRifle
. Например, проверка на то, чтоPlayer
не мертв (dead), перед атаками и состояниямиshoot()
может улучшить устойчивость кода. -
Отсутствие comments и документации: Важно добавлять комментарии к методам и структурами, чтобы любой, кто будет читать ваш код, мог быстро понять логику и намерение.
-
Проверка целостности данных: Можно подумать о внедрении проверок ввода, чтобы избежать того, что пользователь установит невалидные значения здоровья или навыков.
В заключение, рефакторинг данной программы в сторону большей независимости компонентов, использование современных механизмов управления памятью и добавление дополнительных проверок позволят создать более надежное, безопасное и поддерживаемое приложение.