В разных соцсетях я постоянно говорю о том, что самодокументирующийся код -- это миф. Этот тезис сложно защищать, т.к. люди разные, работают в разных условиях, над проектами разной сложности. Кто-то пишет код, обреченный на поддержку и развитие в течении десятилетий, а чьи-то разработки могут быть выброшены в корзину сразу после релиза или даже не доходя до релиза.
Без конкретных примеров вообще сложно рассуждать о "полезных" и "бесполезных" комментариях. А с конкретными примерами не так-то и просто, потому что не будешь же ты публиковать фрагменты кода из закрытых проектов, а именно там и творится самый треш и угар...
Однако, в этот раз я получил разрешение от заказчика на публикацию небольшого фрагмента. Без подробностей, просто в качестве иллюстрации.
Исходно был вот такой кусочек:
|
catch (const std::exception& e) { // если были записаны данные, то нужно обновить все // FIXME: тут пока не будем разделять updateAfterSetCells(std::move(isf), anyUpdated); throw(e); } // обновление таблицы консолидации на основании новых ячеек updateAfterSetCells(InsertionStatus_Facts{ getCubeIndex() }, anyUpdated); return isf; } |
Понятно, что есть принципиальная разница при выходе из функции из-за исключения и при нормальном завершении. Но по самому коду не удавалось установить почему эта разница существует. И, кроме того, закрадывались сомнения в правильности подобного разделения.
Чтобы это выяснить пришлось сперва подергать нескольких коллег в мессенджерах, а потом устроить коллективный созвон на получаса.
В итоге удалось найти и связать концы. А чтобы впредь не заниматься подобными археологическими раскопками, исходные комментарии были заменены вот такими расширенными версиями:
|
catch (...) { // Не важно что за исключение возникло. // // Часть ячеек могла быть изменена/добавлена, и эта часть // должна быть зафиксирована. Если не делать фиксацию, то // часть информации об ячейках может быть потеряна, а // FactsTable останется в непонятном состоянии (т.к. внутри // FactsTable сейчас идет промежуточное накопление новых фактов // для оптимизации их добавления, эти новые факты в промежуточном // буфере внутри FactsTable должны быть сохранены). // // ПРИМЕЧАНИЕ: важно то, что в updateAfterSetCells идет объект // isf, возможно, с актуальной локальной копией FactsTable. // Если эта локальная копия существует, то будут обновлены и // фидеры. Это обновление фидеров должно быть сделано, ведь из-за // выброса исключений не будет обновление фидеров внутри процесса. // // FIXME: тут пока не будем разделять updateAfterSetCells(isf, tableCopy, anyUpdated); throw; } // Обновление таблицы консолидации на основании новых ячеек // // ПРИМЕЧАНИЕ: этот вызов принципиально отличается от аналогичного // в блоке catch потому, что обновление фидеров (если в isf есть актуальная // локальная копия FactsTable) будет явным образом выполняться процессом // выше по стеку. // // Специально отдаем в updateAfterSetCells пустой InsertionStatus_Facts, // чтобы не было обновлений фидеров. InsertionStatus_Facts emptyIsf{ getCubeIndex() }; updateAfterSetCells(emptyIsf, tableCopy, anyUpdated); return isf; } |
Что тоже, конечно же, не идеал. Но что-то подобное хотелось бы видеть изначально, ибо раскапывать подобные нюансы из кода -- ну такое себе удовольствие. И это еще повезло, что проект молодой, состав разработчиков еще не сменился, можно было поговорить с авторами лично, а у них еще детали не стерлись из памяти.
