вторник, 18 июня 2024 г.

[prog] Один из признаков того, что с вашим кодом не все в порядке

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

Т.е. если вы видите в кодовой базе что-то вроде:

data_manager::data_manager(
  data_set_id id,
  raw_data_storage_shptr raw_data_storage,
  access_manager_shptr access_manager,
  modification_mode mode,
  logger log)
  : base_type{ id,
      std::move(raw_data_storage),
//     std::move(access_manager)
//     mode
    }
{
//  _id = id;
//  _storage = std::move(raw_data_storage);
//  _access_manager = std::move(access_manager);
  setup_access_manager(std::move(access_manager));
  setup_mode(mode);
  setup_logger(std::move(log));
}

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

Полагаю, тут работает "закон разбитых окон": когда заглядываешь в код и видишь, что такое является нормой, то и сам невольно поддаешься соблазну поступать аналогичным образом.

А избежать этого не так уж и сложно. Достаточно взять за правило либо сразу же удалять ненужный код, либо же снабжать его комментарием о том, почему закомментирован и когда (при каких условиях) его нужно раскомментировать или удалить.

Еще неплохо бы пометить в тех же комментариях кому нужно будет принять решение (скорее всего это вы сами, так что впишите туда свой никнейм).

Например:

data_manager::data_manager(
  data_set_id id,
  raw_data_storage_shptr raw_data_storage,
  access_manager_shptr access_manager,
  modification_mode mode,
  logger log)
  : base_type{ id,
      std::move(raw_data_storage),
//FIXME(eao197): access_manager и mode должны были бы передаваться
//в base_type, но там временно пришлось убрать такой конструктор.
//Нужно вернуться к передаче access_manager и mode в конструктор
//базового класса после того, как base_type будет отрефакторен.
//     std::move(access_manager)
//     mode
    }
{
  //FIXME(eao197): установка access_manager и mode должна
  //быть со временем перенесена в конструктор base_type.
  setup_access_manager(std::move(access_manager));
  setup_mode(mode);

  setup_logger(std::move(log));
}

PS. Был в моей карьере один случай двухдневного сеанса отладки кода, который до сих пор вспоминается с содроганием. Выглядел этот код как откровенное Г и состоял на 50% вот таких вот закомментированных кусков без всяких пояснений. Причем ладно бы в начале файлов были раскомментированные, актуальные строки, а в конце -- закомментированные, уже не нужные. Так нет, все это было вперемешку. Бррр. Как вспомню, так вздрогну.

5 комментариев:

Stanislav Mischenko комментирует...

Я, бывает, чтобы не захламлять файлы закомментированным кодом, держу в папке проекта что-то вроде "мусорной корзины", куда складываю код, с которым по какой-то причине сразу не расстаться.
P.S. Если что, я в курсе за систему версий, не надо мне рассказывать ;)

XX комментирует...

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

eao197 комментирует...

@XX

У меня зачастую (раз в день, иногда пару раз в день) бывают т.н. "backup commit", т.е. коммиты которые сделаны просто для того, чтобы не потерять то, что есть. Грубо говоря, завершается рабочий день, я фиксирую в репозитории то, что успел сделать (обычно это все в отдельно feature branch). В таких backup commit-ах наличие временно закомментированного кода (как и просто ошметков всякого разного), как по мне, вполне нормально.

Не делать backup commit-ов и хранить исходники в своей рабочей копии несколько дней не могу. В прошлом несколько раз терял результаты работы из-за порчи дискет. Это отложило свой отпечаток.

Меня как раз напрягают разработчики, которые скидывают результаты своего труда в репозиторий раз или два в неделю. Откуда у них такая вера в надежность их техники? ;)))

XX комментирует...

Просто коммитить нужно почаще )

Если изменений много, а задача ещё не доделана, то всё равно можно разбить сделанное на относительно законченные изменения, выделить уже готовые мелочи, которые можно закоммитить. У меня обычно это работает.

XX комментирует...

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