Опять Minesweeper (C++/MFC)

Опубликовано Oct 20, 2012 в Игры, Разбор примеров кода, Хорошие проекты студентов | 5 коммент.



Опять Minesweeper (C++/MFC)

В предыдущем посте про игру “Сапер” был комментарий с предложением варианта решения. Автор переслал его нам. Краткая оценка такая.

Я смотрела только код, связанный с логикой игры (файлы game.h и game.cpp. Все, что связано с интерфейсом на MFC, я комментировать не берусь, ввиду того, что с этой библиотекой никогда не сталкивалась. Достаточно того, что интерфейсная часть работоспособна, а логика игры отделима от интерфейса достаточно прозрачно, что хорошо, и это будет использовано при небольшом рефакторинге кода, о чем и пойдет речь далее.

Общие замечания

0) Стилю написания кода особого внимания не уделялось, как я понимаю. В файлах game.h и game.cpp находится код нескольких разнообразных классов, причем реализация их методов даже не записана по порядку. Имена идентификаторов не единообразны, например, есть имена классов из заглавных букв (классы состояний ячейки CLOSED,  OPENED, MARKED). Вопросы отступов, выравнивания,  наличия пробелов там, где это положено, например, после запятых, были явно пущены на самотек. В целом по этому поводу рекомендуется прочитать, например, “Соглашения по оформлению кода RSDN” и начинать ими пользоваться.

 Общий подход к дизайну логики игры выдержан в объектном стиле, возможно, даже в несколько переусложненном, но по этому коду видно умение применять ООП.

Что я изменила бы

1) Поля класса msw_state (static int msw_state::n_closed;    static int msw_state::n_opened;    static int msw_state::n_marked;) ,  описывающего состояние конкретной клетки, имеют смысл только в контексте конкретной игры (экземпляра класса game). Просто для клетки (экземпляра класса cell) они бессмысленны, вернее одинаковы для всех создаваемых по ходу игры клеток  в каждый конкретный момент. Поэтому их и сделали статическими, а логичнее вообще их перенести в класс game, не объявляя статическими. Кроме того, их объявили публичными, что является нарушением принципа инкапсуляции.

2) Код написан таким образом, что по ходу игры создается и уничтожается большое количество объектов, хранящих статусы ячеек. Таких статусов всего-то имеется три – для их описания достаточно создать по одному объекту классов CLOSED,  OPENED, MARKED и все. А сейчас их создаются и уничтожаются сотни штук – для того, чтобы обеспечить логику подсчета  значений n_closed, n_opened и n_marked, которая в исходном варианте реализована в конструкторах и деструкторах классов статуса. Если вспомнить, что предназначение конструктора – инициализация экземпляра класса, а деструктора – очистка, то идея использовать их для  подсчета чего-либо видится неожиданной.

3) Итак, после перенесения полей n_closed, n_opened и n_marked в класс game, инициализируем их в конструкторе этого класса. Дальше в классе game заводим три поля для описания статусов клеток, инициализируем их, после чего инициализируем сами клетки (массив minefield) со ссылкой на исходный статус CLOSED.

4) В простейших игровых задачах (карты, морской бой и сапер – среди них) при инициализации игры нужно решать задачу генерации случайных перестановок. Предложенный вариант вполне хороший, изменения сделаны, чтобы показать, как убрать лишний вспомогательный массив vector tmp_minefield.

Сображения из пунктов 1) – 4) привели к тому, что наиболее сильно изменился конструктор класса game.

[learn_more  caption = "Конструктор класса  game - первоначальный вариант"]

game::game()
{
msw_state::n_closed=0;
msw_state::n_opened=0;
msw_state::n_marked=0;
in_game=true;
x_cells=9;
y_cells=9;
bombs=10;
 
vector tmp_minefield(x_cells*y_cells-bombs,0);
tmp_minefield.insert(tmp_minefield.end(),bombs,1);
 
srand(unsigned(time(NULL)));
random_shuffle(tmp_minefield.begin(),tmp_minefield.end());
 
 for(int i=0,idx=0;i(minefield).swap(minefield);
 
for_each(minefield.begin(),minefield.end(),
  bind2nd(mem_fun(&cell::on_msg),m_init));
}

[/learn_more]

Конструктор класса game - измененный вариант

game::game()
{
  stateCLOSED = new CLOSED();
  stateOPENED = new OPENED();
  stateMARKED = new MARKED();
 
  n_closed=x_cells*y_cells;
  n_opened=0;
  n_marked=0;
 
  in_game=true;
  x_cells=9;
  y_cells=9;
  bombs=10;
 
  //vector tmp_minefield(x_cells*y_cells-bombs,0);
  //tmp_minefield.insert(tmp_minefield.end(),bombs,1);
 
  for(int i = 0, idx = 0; i < x_cells; ++i)
    for(int j = 0; j < y_cells; ++j, ++idx)
    {
      cell* new_cell;
      if(idx >= x_cells*y_cells-bombs)
      {
        new_cell=new  bomb_cell(this,make_pair(0,0));
      }
      else
      {
        new_cell=new empty_cell(this,make_pair(0,0));
      }
 
      minefield.push_back(new_cell);
    }
 
    srand(unsigned(time(NULL)));
    random_shuffle(minefield.begin(),minefield.end());
 
    for(int i = 0; i < x_cells; ++i)
      for(int j = 0; j < y_cells; ++j)
      {
        minefield[i*x_cells + j]->set_coord(i,j);
      }
 
      for_each(minefield.begin(),minefield.end(),
       bind2nd(mem_fun(&cell::on_msg),m_init));
}

5) По ходу игры в результате манипуляций пользователя статусы клеток будут переключаться просто сменой ссылки на нужный статус, для чего в классе cell изменен метод set_state. Так как смена статуса клетки ведет к изменению итоговых значений для статусов (n_closed, n_opened и n_marked), то в этом же методе они и переcчитываются, а больше n_closed, n_opened и n_marked не могут изменяться нигде. Таким образом, все взаимозависимые изменения и пересчеты собраны в одном единственном месте.

6) Описанные изменения вносились в классы таким образом, чтобы не затронуть сигнатуру методов, на которых построен MFC-интерфейс. Чтобы после сделанных изменений не возникли проблемы с правами доступа, классу game пришлось подарить дружбу другим классам, для чего в его заголовок добавили следующий код

   friend class cell;
   friend class CLOSED;
   friend class OPENED;
   friend class MARKED;

Это все. Если автор не выступит с возражениями – через некоторое время я выложу оба варианта кода целиком. 


Автор публикации: