[prog.flame] Самодокументирующися код против документированного, наглядно

Намедни в LinkedIn поиронизировал на счет "самодокументирующегося кода". В очередной раз 😎

В очередной раз с удивлением для себя обнаружил людей, скептически относящихся к необходимости комментировать код.

Подумал, что лучше одна иллюстрация лучше тысячи слов. Поэтому вот пример кода из реального проекта. Изменены только названия сущностей, все комментарии оставлены как есть. Плюс изъяты фрагменты, не относящиеся к самой иллюстрации.

Каждый сам может для себя решить, с каким кодом ему проще было бы разбираться. Сам-то я для себя уже давно выводы сделал. Поэтому и пишу комментарии, хотя тут мне еще есть куда расти, к сожалению. Не всегда получается сделать хорошо с первого раза 🙁

Под катом оригинальный фрагмент с комментариями. Такими, какими они и были написаны при разработке. Но сперва этот же фрагмент, но вообще без комментариев. Именно так "самодокументирующися" код и выглядит, по моему (не)скромному опыту.

class DefaultThreadPoolScheduler final : public Scheduler {    ... private:    struct WorkerData    {       std::mutex _lock;       std::condition_variable _wakeupCondition;       Scheduler::TaskUniquePtr _taskToRun;       bool _shutdownInitiated{ false };    };    using WorkerDataContainer =       std::vector<std::reference_wrapper<WorkerData>>;    std::latch _allWorkersStartedLatch;    std::mutex _lock;    TasksContainer _tasksQueue;    ThreadPool _threadPool;    WorkerDataContainer _availableWorkers;    bool _shutdown{ false }; }; void DefaultThreadPoolScheduler::doWork() noexcept {    WorkerData thisWorkerData;    {       std::lock_guard schedulerLock{ _lock };       _availableWorkers.push_back(std::ref(thisWorkerData));       _allWorkersStartedLatch.count_down();    }    bool shutdownIntitiated{ false };    while( !shutdownIntitiated )    {       std::unique_lock workerLock{ thisWorkerData._lock };       if( TaskUniquePtr taskToRun = std::move(thisWorkerData._taskToRun); !taskToRun )       {          shutdownIntitiated = thisWorkerData._shutdownInitiated;          if( !shutdownIntitiated )          {             thisWorkerData._wakeupCondition.wait(workerLock);             shutdownIntitiated = thisWorkerData._shutdownInitiated;          }       }       else       {          workerLock.unlock();          taskToRun->run(Scheduler::RunCondition::Normal);          completeTaskThenTryGetNext(std::move(taskToRun), thisWorkerData);       }    } }

Ну а вот исходник с комментариями:

class DefaultThreadPoolScheduler final : public Scheduler {    ... private:    /// Описание одной свободной рабочей нити.    ///    /// Каждая рабочая нить создает такой объект у себя на стеке,    /// а объект-шедулер хранит у себя ссылки на эти объекты.    struct WorkerData    {       /// Замок для обеспечения thread-safety.       std::mutex _lock;       /// Для пробуждения рабочей нити когда для нее есть task или       /// же когда нужно завершить работу.       std::condition_variable _wakeupCondition;       /// Task который должен быть запущен на контексте       /// этой рабочей нити.       ///       /// Пустое значение указывает, что в данный момент Task-а нет.       Scheduler::TaskUniquePtr _taskToRun;       /// Признак необходимости завершить работу.       ///       /// Если выставлен в true, то больше никаких действий рабочая       /// нить предпринимать не должна.       bool _shutdownInitiated{ false };    };    /// Тип контейнера с информацией о свободных рабочих нитях.    using WorkerDataContainer =       std::vector<std::reference_wrapper<WorkerData>>;    /// Барьер для синхронизации запуска рабочих нитей.    ///    /// Нужен для того, чтобы после запуска thread-pool-а убедиться в том,    /// что все рабочие нити успели войти в doWork и добавили себя    /// в _availableWorkers. Если этого не сделать, то может случиться так,    /// что конструктор DefaultThreadPoolScheduler завершится раньше, чем на    /// рабочих нитях произойдет вход в doWork. В этом случае мы сможем    /// начать получать новые заявки еще до того, как во _availableWorkers    /// появится информация о свободных рабочих нитях. А это приведет к тому,    /// что запросы будут становиться в очередь, но извлекаться оттуда не будут.    std::latch _allWorkersStartedLatch;    /// Замок объекта для обеспечения thread safety.    std::mutex _lock;    /// Очередь запросов.    TasksContainer _tasksQueue;    /// Пул рабочих потоков для обслуживания запросов.    ThreadPool _threadPool;    /// Список свободных рабочих нитей.    WorkerDataContainer _availableWorkers;    /// Признак того, что работа шедулера была завершена.    bool _shutdown{ false }; }; void DefaultThreadPoolScheduler::doWork() noexcept {    WorkerData thisWorkerData;    // Изначально эта нить свободна и шедулер должен об этом узнать.    {       std::lock_guard schedulerLock{ _lock };       _availableWorkers.push_back(std::ref(thisWorkerData));       // Информируем, что еще одна нить внесла себя в _availableWorkers.       // Если этого не сделать, то конструктор DefaultThreadPoolScheduler не       // завершит свою работу.       _allWorkersStartedLatch.count_down();    }    bool shutdownIntitiated{ false };    while( !shutdownIntitiated )    {       std::unique_lock workerLock{ thisWorkerData._lock };       if( TaskUniquePtr taskToRun = std::move(thisWorkerData._taskToRun); !taskToRun )       {          // Если оказались здесь, значит очередного запроса для обработки нет,          // нужно его подождать, но делать это можно только если не начали shutdown.          shutdownIntitiated = thisWorkerData._shutdownInitiated;          if( !shutdownIntitiated )          {             // Спонтанные пробуждения здесь не страшны. Если нас             // случайно разбудили, то на следующей итерации мы просто             // уснем еще раз.             thisWorkerData._wakeupCondition.wait(workerLock);             // Еще раз обновляемся, т.к. shutdown мог начаться пока мы ждали.             shutdownIntitiated = thisWorkerData._shutdownInitiated;          }       }       else       {          // Выполнять заявку нужно при незаблокированном замке рабочей нити,          // в противном случае получим проблемы в completeTaskThenTryGetNext.          workerLock.unlock();          taskToRun->run(Scheduler::RunCondition::Normal);          completeTaskThenTryGetNext(std::move(taskToRun), thisWorkerData);       }    } }