﻿# Проверка игрового движка qdEngine, часть первая: топ 10 предупреждений PVS\-Studio

Баги, которые удалось найти в движке qdEngine, оказались весьма разнообразны, поэтому не хочется мешать всё в кучу в одной публикации\. Читатели могут упустить интересные темы, связанные с написанием качественного кода\. Поэтому разбор проекта выйдет в виде серии публикаций, первая из которых посвящается наиболее интересным срабатываниям с точки зрения плагина PVS\-Studio\.

![1119_qdEngine_check_ru/image1.png](https://import.viva64.com/docx/blog/1119_qdEngine_check_ru/image1.png)

## Кнопка Best

Ассоциация K\-D Lab открыла исходный код игрового движка [qdEngine](https://github.com/KD-lab-Open-Source/qdEngine), предназначенного для создания квестов \([новость на сайте opennet](https://www.opennet.ru/opennews/art.shtml?num=60752)\)\. Движок в основном написан на языке C\+\+ и выглядит интересным объектом для исследования с помощью анализатора кода PVS\-Studio\.

И действительно, как можно пройти мимо движка, на основе которого в своё время были созданы такие игры, как:

* Братья Пилоты 3D\. Дело об Огородных вредителях;
* Братья Пилоты 3D\-2\. Тайны Клуба Собаководов;
* Братья Пилоты\. Обратная сторона Земли;
* Ну, погоди\! Выпуск 3\. Песня для зайца;
* Похождения бравого солдата Швейка\.

Ошибки, которые удалось найти в проекте, разнообразны, и их хочется рассмотреть с разных точек зрения\. Поэтому будет несколько публикаций на разные темы, и первая из них посвящается кнопке **Best** в плагинах [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/)\.

![1119_qdEngine_check_ru/image2.png](https://import.viva64.com/docx/blog/1119_qdEngine_check_ru/image2.png)

Кнопка помогает пользователю при первом знакомстве с инструментом PVS\-Studio\. Анализатор выбирает 10 предупреждений, которые, скорее всего, должны указывать на реальные ошибки и при этом быть разнообразными и интересными\.

Все статические анализаторы кода при первом запуске выдают большое количество предупреждений, многие из которых, скорее всего, окажутся неактуальными или ложными\. Это не страшно, так как анализаторы, в том числе и PVS\-Studio, имеют гибкие настройки\.

Однако программист, который только знакомится с методологией статического анализа, может быть демотивирован множеством предупреждений, которые для него выглядят неактуальными\. Хочется сразу увидеть какие\-то интересные ошибки\. Кнопка **Best** как раз позволяет это сделать\.

При нажатии на неё анализатор по эмпирическому алгоритму выбирает 10 предупреждений\. При выборе он руководствуется не только вероятностью, что перед нами именно ошибка, но и старается выбрать предупреждения разного типа\.

<details>
   <summary>Пояснение про достоверность ошибок\\\.</summary>

Стоит объяснить, почему выбрать 10 предупреждений, которые точно являются ошибкой, не самый лучший вариант\.

Для примера рассмотрим диагностику [V668](https://pvs-studio.ru/ru/docs/warnings/v668/)\. Она сообщает о бессмысленной проверке указателя, который вернул оператор _new_\. Пример реального кода из проекта Minetest:

```cpp
clouds = new Clouds(smgr, -1, time(0));
if (!clouds) {
 *error_message = "Memory allocation error (clouds)";
  errorstream << *error_message << std::endl;
  return false;
}
```

Если анализатор нашёл подобный код, то перед нами ошибка\. Конечно, ещё существует _nothrow_\-версия оператора _new_, но анализатор это тоже учитывает\.

Таких ошибок много\. На текущий момент при написании статей мы нашли уже 1630 подобных багов в открытых проектах\! С большой вероятностью подобные фрагменты будут найдены и в проекте, который используется для знакомства с анализатором\.

Однако выдавать много подобных предупреждений не хочется:

1. Это неинтересно\. Не хочется смотреть, например, на семь одинаковых предупреждений про _new_\.
1. Достоверность не означает критичность\. Это как раз такой случай\. Перед нами фрагмент недостижимого кода\. Это не хорошо, но почти наверняка приложение выполняет свои функции\. Интересные и критические ошибки обычно выявляются более сложными диагностическими правилами\. Чем сложнее ошибка, которую мы ищем, тем труднее гарантировать, что найден именно баг\. Даже человеку приходится поломать голову в таких местах\.


</details>
Для проекта qdEngine эта кнопка отработала хорошо:

* восемь предупреждений указывают на баги или неудачный код;
* одно предупреждение формально правильно, но реальной ошибки нет;
* одно предупреждение ложное\.

На мой взгляд, все эти предупреждения хорошо демонстрируют возможности анализатора, и сейчас мы их рассмотрим\. Надеюсь, это подтолкнёт тех, кто только читал про PVS\-Studio, опробовать его в деле\. [Скачивайте](https://pvs-studio.ru/ru/pvs-studio/download/), устанавливайте, проверяйте проект и нажимайте кнопку **Best**\. Если что\-то пойдёт не так, то просто напишите нам в поддержку\. Всегда подскажем и поможем\.

## Предупреждения

Рассмотрим обещанные предупреждения и заодно кое\-где поупражняемся в рефакторинге\.

### Предупреждение N1: дубликат

```cpp
bool qdGameObject::init()
{
  ....
  drop_flag(QD_OBJ_STATE_CHANGE_FLAG | QD_OBJ_IS_IN_TRIGGER_FLAG |
            QD_OBJ_STATE_CHANGE_FLAG | QD_OBJ_IS_IN_INVENTORY_FLAG);
  ....
}
```

Предупреждение PVS\-Studio: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions 'QD\_OBJ\_STATE\_CHANGE\_FLAG' to the left and to the right of the '\|' operator\. qd\_game\_object\.cpp 176

Дважды используется константа _QD\_OBJ\_STATE\_CHANGE\_FLAG_, объявленная в файле _qd\_game\_object\.h_ рядом со многими другими:

```cpp
....
const int QD_OBJ_HAS_BOUND_FLAG           = 0x80;
const int QD_OBJ_DISABLE_MOVEMENT_FLAG    = 0x100;
const int QD_OBJ_DISABLE_MOUSE_FLAG       = 0x200;
const int QD_OBJ_IS_IN_TRIGGER_FLAG       = 0x400;
const int QD_OBJ_STATE_CHANGE_FLAG        = 0x800;
const int QD_OBJ_IS_IN_INVENTORY_FLAG     = 0x1000;
const int QD_OBJ_KEYBOARD_CONTROL_FLAG    = 0x2000;
....
```

Скорее всего, забыли использовать какую\-то другую константу для составления маски\. Возможно, это просто дубликат, и повторяющуюся константу можно удалить из выражения\. Точно мне тут сложно сказать, так как я не знаком с кодом проекта\.

### Предупреждение N2: опечатка \(потерянный символ \#\)

Очень красивая, на мой взгляд, ошибка\. Согласен, у меня странный вкус и профессиональная деформация, но, надеюсь, вы тоже оцените этот баг\.

```cpp
const qdGameObjectStateWalk* qdGameObjectMoving::current_walk_state() const
{
  const qdGameObjectState* st = get_cur_state();
  if(!st || st -> state_type() != qdGameObjectState::STATE_WALK){
#ifndef _QUEST_EDITOR
    st = last_walk_state_;
    if(!st || st -> state_type() != qdGameObjectState::STATE_WALK)
      st = get_default_state();
else
    st = get_default_state();
    if(!st) st = get_state(0);
#endif
  }
  ....
}
```

Предупреждение PVS\-Studio: [V523](https://pvs-studio.ru/ru/docs/warnings/v523/) The 'then' statement is equivalent to the 'else' statement\. qd\_game\_object\_moving\.cpp 2781

Анализатор обращает внимание, что, независимо от условия, будет выполнено одно и то же действие:

```cpp
  if(!st || st -> state_type() != qdGameObjectState::STATE_WALK)
    st = get_default_state();
else
  st = get_default_state();
```

Обратите внимание, что код как\-то странно отформатирован\. Я имею в виду, что ключевое слово _else_ находится в начале строки\.

Если посмотреть на код рядом и немного подумать, то становится ясно, что на самом деле хотели написать не оператор _else_, а директиву препроцессора _\#else_\. Однако программист опечатался и забыл символ решётки \(_\#_\)\.

Исправленный код:

```cpp
const qdGameObjectState* st = get_cur_state();
if(!st || st -> state_type() != qdGameObjectState::STATE_WALK){
#ifndef _QUEST_EDITOR
  st = last_walk_state_;
  if(!st || st -> state_type() != qdGameObjectState::STATE_WALK)
    st = get_default_state();
#else
  st = get_default_state();
  if(!st) st = get_state(0);
#endif
```

### Предупреждение N3: неиспользуемая строка

```cpp
bool qdGameScene::adjust_files_paths(....)
{
  ....
  for(qdGameObjectList::const_iterator it = object_list().begin();
      it != object_list().end(); ++it)
  {
    if((*it) -> named_object_type() == QD_NAMED_OBJECT_STATIC_OBJ)
    {
      qdGameObjectStatic* obj = static_cast<qdGameObjectStatic*>(*it);
      if(obj -> get_sprite() -> file())
        QD_ADJUST_TO_REL_FILE_MEMBER(pack_corr_dir, 
                                     obj -> get_sprite() -> file, 
                                     obj -> get_sprite() -> set_file, 
                                     can_overwrite, 
                                     all_ok);  
      std::string str = obj -> get_sprite() -> file();
      str = str;
    }
  }
  ....
}
```

Предупреждение PVS\-Studio: [V570](https://pvs-studio.ru/ru/docs/warnings/v570/) The 'str' variable is assigned to itself\. qd\_game\_scene\.cpp 1799

В цикле происходит какая\-то работа со списком файлов\. Как я понимаю, в целях отладки был добавлен этот фрагмент кода, который и не нравится анализатору:

```cpp
std::string str = obj -> get_sprite() -> file();
str = str;
```

В первой строчке берётся имя файла\. Вторая строчка, на мой взгляд, служит для двух целей:

1. На неё удобно поставить точку останова в отладчике и сразу посмотреть имя файла в переменной _str_\.
1. Присваивание объекта самому себе может дополнительно убирать предупреждение какого\-то компилятора/анализатора о том, что переменная _str_ не используется\.

Ошибки здесь нет\. Однако это код с запахом, и его стоит улучшить\. Другими словами, это предупреждение анализатора — хороший повод заняться рефакторингом\. Для начала я бы изменил интерфейс функции _file_ в классе _qdSprite_:

```cpp
const char* file() const { return file_.c_str(); }
```

Странно возвращать содержимое _std::string_ как просто указатель, а затем ещё и проверять его в разных местах на _nullptr_\. На самом деле, функция никогда не вернёт _nullptr_\. Если же где\-то в коде нужен именно указатель, то всегда можно будет в этом месте вызвать _c\_str_\. Изменённая функция:

```cpp
const std::string& file() const { return file_; }
```

Естественно, изменение интерфейса функции затронет много мест в коде проекта\. Тем более, что для симметричности есть смысл изменить и функцию _set\_file_\. Однако у меня есть подозрение, что всё это пошло бы коду на пользу\.

Продолжим\. Перепишем рассмотренный ранее фрагмент кода\.

```cpp
if((*it) -> named_object_type() == QD_NAMED_OBJECT_STATIC_OBJ)
{
  qdGameObjectStatic* obj = static_cast<qdGameObjectStatic*>(*it);
  const std::string &file = obj -> get_sprite() -> file();
  QD_ADJUST_TO_REL_FILE_MEMBER(pack_corr_dir, 
                               file, 
                               obj -> get_sprite() -> set_file, 
                               can_overwrite, 
                               all_ok);  
}
```

Улучшения:

1. Код стал короче и проще\.
1. Исчезла бессмысленная проверка указателя\.
1. Нет создания временного объекта _std::string_ в цикле только для того, чтобы было проще отлаживаться\. Это относительно ресурсоёмкие операции из\-за выделения и освобождения памяти\. При этом нет гарантии, что компилятор в процессе оптимизации удалит всё лишнее\. 
1. Анализатор PVS\-Studio больше не выдаёт предупреждение [V570](https://pvs-studio.ru/ru/docs/warnings/v570/)\.
1. В целях отладки всё так же легко поставить точку останова \(на строчку с макросом _QD\_ADJUST\_TO\_REL\_FILE\_MEMBER_\) и посмотреть имя файла\.

### Предупреждение N4: потенциальное использование нулевого указателя

```cpp
bool DDraw_grDispatcher::Finit()
{
  ....
  ddobj_ -> SetCooperativeLevel((HWND)Get_hWnd(),DDSCL_NORMAL);
  if(fullscreen_ && ddobj_) ddobj_ -> RestoreDisplayMode();
  ....
}
```

Предупреждение PVS\-Studio: [V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'ddobj\_' pointer was utilized before it was verified against nullptr\. Check lines: 211, 212\. ddraw\_gr\_dispatcher\.cpp 211

Указатель _ddobj_ разыменовывается для вызова функции\. При этом ниже есть проверка этого указателя\. На основании этой проверки анализатор делает вывод, что переменная _ddobj_ может содержать _nullptr_, о чём и предупреждает нас\.

Код можно переписать, например так:

```cpp
if (ddobj_)
{
  ddobj_ -> SetCooperativeLevel((HWND)Get_hWnd(),DDSCL_NORMAL);
  if(fullscreen_) ddobj_ -> RestoreDisplayMode();
}
```

### Предупреждение N5: деструктор не является виртуальным

```cpp
template <class T>
class PtrHandle 
{
  ....
  ~PtrHandle() { delete ptr; }
  ....
private:
  T *ptr;
};
```

Предупреждение PVS\-Studio: [V599](https://pvs-studio.ru/ru/docs/warnings/v599/) Instantiation of PtrHandle < ResourceUser \>: The virtual destructor is not present, although the 'ResourceUser' class contains virtual functions\. Handle\.h 14

В коде класса _PtrHandle_ ошибки нет\. Однако проблема в том, что в нём хранится объект типа _ResourceUser_\. Посмотрим на объявление этого класса:

```cpp
class ResourceUser
{        
  int ID;
  static int IDs;
public:
  ResourceUser(time_type period) { dtime = period; time = 0; ID = ++IDs; }
  virtual int quant() { return 1; } 

protected:
  time_type time;
  time_type dtime;  

  virtual void init_time(time_type time_) { time = time_ + time_step(); } 
  virtual time_type time_step() { return dtime; } 

  friend class ResourceDispatcher;
};
```

В этом классе есть виртуальные функции\. Следовательно, предполагается, что от этого класса будут наследоваться другие классы\. А раз так, то и деструктор следует объявить как виртуальный\. В противном случае деструктор класса _PtrHandle_ будет не полностью разрушать объекты, которые наследуются от _ResourceUser_\. Это приводит к неопределённому поведению и утечкам ресурсов\.

### Предупреждение N6: неверный оператор delete

```cpp
char* grDispatcher::temp_buffer(int size)
{
  if(size <= 0) size = 1;

  if(size > temp_buffer_size_){
    delete temp_buffer_;
    temp_buffer_ = new char[size];
    temp_buffer_size_ = size;
  }

  return temp_buffer_;
}
```

Предупреждение PVS\-Studio: [V611](https://pvs-studio.ru/ru/docs/warnings/v611/) The memory was allocated using the 'operator new\[\]' but was released using the 'operator delete'\. The 'delete\[\] temp\_buffer\_;' statement should be used instead\. Check lines: 1241, 1242\. gr\_dispatcher\.cpp 1241

В переменной _temp\_buffer\__ хранятся указатели на массивы, созданные с помощью оператора _new\[\]_\. Следовательно, уничтожаться они должны с помощью оператора _delete\[\]_\.

В рассматриваемом коде для уничтожения массива используется просто _delete_, предназначенный для одиночных объектов\. В результате этот код приводит к неопределённому поведению\.

Примечание\. Кто\-то может возразить, что на практике всё будет работать, так как массив состоит из элементов типа _char_, и работа операторов _new_/_delete_ сводится к вызову функций _malloc_/_free_\. [Это не так](https://pvs-studio.ru/ru/blog/posts/cpp/0973/)\. Реализация операторов может быть весьма разнообразной\. Например, с целью оптимизации одиночные объекты и массивы могут создаваться в различных заранее выделенных \(зарезервированных\) пулах памяти\.

### Предупреждение N7: ошибочный тип переменной для синхронизации

```cpp
static int b_thread_must_stop=0;

void MpegDeinitLibrary()
{
  ....
  if (hThread!=INVALID_HANDLE_VALUE)
  {
    b_thread_must_stop=1;
    while(b_thread_must_stop==1)
      Sleep(10);
  }
  ....
}
```

Предупреждение PVS\-Studio: [V712](https://pvs-studio.ru/ru/docs/warnings/v712/) Be advised that compiler may delete this cycle or make it infinity\. Use volatile variable\(s\) or synchronization primitives to avoid this\. PlayOgg\.cpp 293

Для синхронизации потоков используется обыкновенная глобальная переменная\. Это плохая идея, так как компилятор в процессе оптимизации может сделать вывод, что она не изменяется, и создаст вечный цикл\. Вдобавок изменение переменной может быть не атомарным\.

В таком случае следует заменить тип _int_ на [_atomic\_int_](https://en.cppreference.com/w/cpp/atomic/atomic):

```cpp
#include <atomic>
static atomic_int b_thread_must_stop { 0 };
```

### Предупреждение N8: утечка памяти

```cpp
bool qdAnimationMaker::insert_frame(....)
{
  // IMPORTANT(pabdulin): auto_ptr usage was removed
  qdAnimationFrame* fp = new qdAnimationFrame;
  fp -> set_file(fname);
  fp -> set_length(default_frame_length_);

  if (!fp -> load_resources())
    return false;
  ....
  delete fp;
  return true;
}
```

Предупреждение PVS\-Studio: [V773](https://pvs-studio.ru/ru/docs/warnings/v773/) The function was exited without releasing the 'fp' pointer\. A memory leak is possible\. qd\_animation\_maker\.cpp 40

Если произойдёт ошибка загрузки ресурсов, то функция досрочно завершит работу\. При этом не будет удалён объект, указатель на который хранится в переменной _fp_\. Произойдёт утечка памяти\.

Правильный вариант:

```cpp
if (!fp -> load_resources())
{
  delete fp;
  return false;
}
```

Впрочем, код даже после исправления ошибки нельзя назвать хорошим\. Ручное управление памятью провоцирует ошибки, что мы здесь и наблюдаем\. Намного лучше использовать умные указатели\.

Интересный момент\. Обратите внимание на комментарий:

```cpp
// IMPORTANT(pabdulin): auto_ptr usage was removed
```

Возможно, когда\-то этот код был правильным, пока в нём использовался умный указатель типа [_std::auto\_ptr_](https://en.cppreference.com/w/cpp/memory/auto_ptr)\. Затем этот класс устарел\. Вместо того, чтобы заменить его на [_std::unique\_ptr_](https://en.cppreference.com/w/cpp/memory/unique_ptr), программист решил работать с указателями "в ручном режиме", но не справился с задачей\.

### Предупреждение N9: ложное срабатывание

```cpp
class PtrHandle 
{
  ....
  PtrHandle& operator=(PtrHandle& p) 
  { 
    if (get() != p.get()) 
    { 
      delete ptr; 
      ptr = p.release(); 
    } 
    return *this; 
  }
  ....

  T* get() const { return ptr; }
  ....
private:
  T *ptr;
};
```

Предупреждение PVS\-Studio: [V794](https://pvs-studio.ru/ru/docs/warnings/v794/) The assignment operator should be protected from the case of 'this \=\= &p'\. Handle\.h 19

Это единственное из 10 предупреждений, выбранных анализатором, которое оказалось ложным\. Диагностика [V794](https://pvs-studio.ru/ru/docs/warnings/v794/) предупреждает, что объект должен быть защищён от копирования в самого себя\. Для этого анализатор ищет в теле функции конструкцию вида:

```cpp
if (this != &p)
```

Но здесь написана более сложная проверка через вызов функций _get_:

```cpp
if (get() != p.get())
```

Возможно, автор хотел защититься не просто от копирования умного указателя в самого себя, а от копирования двух разных умных указателей, ссылающихся на один объект\. Впрочем, в этом нет смысла\. Если два умных указателя \(без подсчёта ссылок\) ссылаются на один объект, то это беда\. Возможна ситуация двойного уничтожения объекта\.

Так что такая проверка на самом деле ничем не лучше\. Она лишь дополнительно запутывает анализатор и людей, читающих код\.

Что ж, анализатор не разобрался в ситуации\. Чтобы устранить ложное предупреждение, можно поступить одним из следующих способов\.

**Вариант 1\.** Добавить комментарий для подавления предупреждения\.

```cpp
PtrHandle& operator=(PtrHandle& p) 
{ //-V794
  if (get() != p.get())
```

Комментарий явно указывает анализатору PVS\-Studio, что код безопасен\.

**Вариант 2\.** Переписать проверку\.

```cpp
if (this != &p)
```

Мне, пожалуй, больше нравится этот способ\. Вариант с вызовом функций, на мой взгляд, смотрится тяжеловесно\.

Если хочется больше контроля, то можно добавить вот такую проверку:

```cpp
PtrHandle& operator=(PtrHandle& p) 
{ 
  if (this != &p)
  { 
    if (get() == p.get())
    {
       // Всё плохо. Один объект в двух умных указателях.
       // Время отлаживаться.
       assert(false);
       throw std::logic_error("Multiple ownership of an object.");
    }
    delete ptr; 
    ptr = p.release(); 
  } 
  return *this; 
}
```

Проверка поможет выявить ситуацию, когда два умных указателя вдруг будут контролировать один и тот же объект\.

### Предупреждение N10: повторное присваивание

```cpp
bool qdGameObjectMoving::update_screen_pos()
{
  ....
  if(get_cur_state() -> state_type() == qdGameObjectState::STATE_WALK){
    qdGameObjectStateWalk::OffsetType offs_type=
      qdGameObjectStateWalk::OFFSET_WALK;               // <=

    switch(movement_mode_){
    case MOVEMENT_MODE_STOP:
      offs_type = qdGameObjectStateWalk::OFFSET_STATIC;
      break;
    case MOVEMENT_MODE_TURN:
      offs_type = qdGameObjectStateWalk::OFFSET_STATIC;
      break;
    case MOVEMENT_MODE_START:
      offs_type = qdGameObjectStateWalk::OFFSET_START;
      break;
    case MOVEMENT_MODE_MOVE:
      offs_type = qdGameObjectStateWalk::OFFSET_WALK;   // <=
      break;
    case MOVEMENT_MODE_END:
      offs_type = qdGameObjectStateWalk::OFFSET_END;
      break;
    }

    offs += static_cast<qdGameObjectStateWalk*>(
      get_cur_state()) -> center_offset(direction_angle_, offs_type);
  }
  ....
}
```

Предупреждение PVS\-Studio: [V1048](https://pvs-studio.ru/ru/docs/warnings/v1048/) The 'offs\_type' variable was assigned the same value\. qd\_game\_object\_moving\.cpp 1094

Анализатору не нравится, что в ветке _MOVEMENT\_MODE\_MOVE_ переменной _offs\_type_ присваивается значение, которое и так в ней находится\.

Формально анализатор прав\. Повторное присваивание не имеет смысла и иногда является не просто лишним кодом, а ошибкой\.

В этом случае бага нет\. Рассмотрим, как можно избавиться от предупреждения\.

**Вариант 1\.** Добавить комментарий _//\-V1048_ для подавления предупреждения\.

**Вариант 2\.** Немного переписать код, добавив _default_\-ветку:

```cpp
if(get_cur_state() -> state_type() == qdGameObjectState::STATE_WALK){
  qdGameObjectStateWalk::OffsetType offs_type;
  switch(movement_mode_){
  case MOVEMENT_MODE_STOP:
    offs_type = qdGameObjectStateWalk::OFFSET_STATIC;
    break;
  case MOVEMENT_MODE_TURN:
    offs_type = qdGameObjectStateWalk::OFFSET_STATIC;
    break;
  case MOVEMENT_MODE_START:
    offs_type = qdGameObjectStateWalk::OFFSET_START;
    break;
  case MOVEMENT_MODE_MOVE:
    offs_type = qdGameObjectStateWalk::OFFSET_WALK;
    break;
  case MOVEMENT_MODE_END:
    offs_type = qdGameObjectStateWalk::OFFSET_END;
    break;
  default:
    offs_type = qdGameObjectStateWalk::OFFSET_WALK;
  }

  offs += static_cast<qdGameObjectStateWalk*>(
    get_cur_state()) -> center_offset(direction_angle_, offs_type);
}
```

Предупреждение исчезнет, но мне не нравится, что переменная _offs\_type_ теперь не инициализируется в месте объявления\. Сделаем ещё один шаг рефакторинга и вынесем часть кода в отдельную функцию\.

```cpp
qdGameObjectStateWalk::OffsetType qdGameObjectMoving::GetOffsetType()
{
  switch(movement_mode_){
  case MOVEMENT_MODE_STOP:    return qdGameObjectStateWalk::OFFSET_STATIC;
  case MOVEMENT_MODE_TURN:    return qdGameObjectStateWalk::OFFSET_STATIC;
  case MOVEMENT_MODE_START:   return qdGameObjectStateWalk::OFFSET_START;
  case MOVEMENT_MODE_MOVE:    return qdGameObjectStateWalk::OFFSET_WALK;
  case MOVEMENT_MODE_END:     return qdGameObjectStateWalk::OFFSET_END;
  }
  return qdGameObjectStateWalk::OFFSET_WALK;
}
....
if(get_cur_state() -> state_type() == qdGameObjectState::STATE_WALK){
  qdGameObjectStateWalk::OffsetType offs_type = GetOffsetType();
  offs += static_cast<qdGameObjectStateWalk*>(
    get_cur_state()) -> center_offset(direction_angle_, offs_type);}
....
```

Код стал короче и при этом легко читается\. Отлично\. Более того, теперь переменная _offs\_type_ вообще не нужна, и код можно ещё немного сократить\.

```cpp
if(get_cur_state() -> state_type() == qdGameObjectState::STATE_WALK){
  offs += static_cast<qdGameObjectStateWalk*>(
    get_cur_state()) -> center_offset(direction_angle_, GetOffsetType());
}
```

Хороший пример: предупреждение анализатора подтолкнуло нас к рефакторингу кода, благодаря чему он стал проще и лучше\.

## Заключение

В статье было показано:

1. Как может помочь кнопка **Best** при первом знакомстве с анализатором PVS\-Studio\. Предлагаю, не откладывая, [получить триал](https://pvs-studio.ru/ru/pvs-studio/try-free/) и попробовать проанализировать свои проекты\.
1. Какие ошибки PVS\-Studio помогает найти и устранить ещё на этапе написания кода\. Чем раньше ошибка найдена, тем проще и дешевле её исправление\.
1. Как предупреждения анализатора подталкивают к улучшению кода \(рефакторингу\)\.

В последующих статьях о qdEngine я разберу ещё ряд ошибок, найденных в проекте\. Спасибо за внимание\.

## Дополнительные ссылки

1. [Развитие инструментария С\+\+ программистов: статические анализаторы кода](https://pvs-studio.ru/ru/blog/posts/cpp/0873/)\.
1. [Предупреждения помогают писать лаконичный код](https://pvs-studio.ru/ru/blog/posts/cpp/0968/)\.
1. [Статический анализатор подталкивает писать чистый код](https://pvs-studio.ru/ru/blog/posts/cpp/1115/)\.
1. [Вызов виртуальных функций в конструкторах и деструкторах \(C\+\+\)](https://pvs-studio.ru/ru/blog/posts/cpp/0891/)\.
1. [Как внедрить статический анализатор кода в legacy проект и не демотивировать команду](https://pvs-studio.ru/ru/blog/posts/0743/)\.