[prog] Наглядный пример роста объема функции при эволюции кода

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

Речь о том, что далеко не всегда объем функций/методов удается держать под жестким контролем. Даже мне, давнему фанату принципа, что код функции должен умещаться на один экран. В качестве примера приведу код функции, над которой как раз таки мне самому довелось поработать. Сначала то, что было изначально. Затем то, что получилось после начала эксплуатации, исправлении одной серьезной проблемы и добавления новых хотелок после накопления некоторого опыта эксплуатации. Код из реального проекта, поэтому все комментарии удалены.

Для чего я это показываю? Для того, чтобы продемонстрировать, что при эволюции кода разработчик может столкнуться с проблемами, о которых он не думал, и с необходимостью добавления функциональности, о которой раньше не было и речи. В результате чего код начинает распухать и далеко не всегда понятно, где именно тот момент, когда одну функцию-переросток нужно разбить на несколько функций поменьше. И, что еще важно, пока этот момент не наступил, объем выполняемой старой функцией-переростком растет, а это сказывается на сложности добавления новой функциональности в функцию (а так же внесения правок в нее).

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

Итак, вот с чего все начиналось:

authsubsys_auth_result_t authsubsys_t::complete_denied_auth(       const steady_clock::time_point now,       int authfunc_result,       clientparam * client,       key_t client_key) {    if(RES_CODE_AUTH_DENY != authfunc_result)       return authsubsys_auth_failed;    auto it = clients_.find(client_key);    if(it == clients_.end()) {       const auto ins_result = clients_.emplace(             std::move(client_key),             user_info_t{                   now + allowed_time_window_,                   not_authentificated_user_t{max_failed_attempts_}});       it = ins_result.first;    }    auto & user_info = it->second;    auto * auth_info = &(get<not_authentificated_user_t>(user_info.info_));    auth_info->failed_attemps_timestamps_.push_back(now);    if(max_failed_attempts_ == auth_info->failed_attemps_timestamps_.size()) {       if(auth_info->failed_attemps_timestamps_.front() + allowed_time_window_ >= now) {          user_info.expires_at_ = now + ban_period_;          user_info.info_ = banned_user_t{};       }       else {          auth_info->failed_attemps_timestamps_.erase(                auth_info->failed_attemps_timestamps_.begin());       }    }    if(!is_banned_user(user_info)) {       user_info.expires_at_ = now + allowed_time_window_;    }    return authsubsys_auth_denied; }

А вот что стало вскоре после того, как этот код стали активно эксплуатировать:

authsubsys_auth_result_t authsubsys_t::complete_denied_auth(       const steady_clock::time_point now,       int authfunc_result,       clientparam * client,       const client_ip_t & client_ip,       key_t client_key) {    AUTHSUBSYS_TRACE(std::cout << "complete_denied_auth:"          << client_key << ", ip=" << client_ip          << ", authfunc_result=" << authfunc_result << std::endl);    if(RES_CODE_AUTH_FAILED == authfunc_result) {       AUTHSUBSYS_TRACE(std::cout << "authentification failure. "             "no actual deny actions" << std::endl);       stats_counters_update(STATS_COUNTER_AUTH_FAILURE);       return authsubsys_auth_failed;    }    try_update_client_ip_status(now, client_ip);    update_stats_counter_for_rejected_auth(client_key.auth_type_);    auto it = clients_.find(client_key);    if(it == clients_.end()) {       const auto ins_result = clients_.emplace(             std::move(client_key),             user_info_t{                   now + allowed_time_window_,                   not_authentificated_user_t{max_failed_attempts_}});       it = ins_result.first;    }    auto & user_info = it->second;    if(is_banned(user_info))       return authsubsys_auth_denied;    else if(is_authentificated_user(user_info))       return authsubsys_auth_failed;    auto * auth_info = &(get<not_authentificated_user_t>(user_info.info_));    auth_info->failed_attemps_timestamps_.push_back(now);    if(max_failed_attempts_ == auth_info->failed_attemps_timestamps_.size()) {       if(auth_info->failed_attemps_timestamps_.front() + allowed_time_window_ >= now) {          AUTHSUBSYS_TRACE(std::cout << "user banned: " << client_key << std::endl);          user_info.expires_at_ = now + ban_period_;          user_info.info_ = banned_user_t{};       }       else {          auth_info->failed_attemps_timestamps_.erase(                auth_info->failed_attemps_timestamps_.begin());       }    }    if(!is_banned(user_info)) {       user_info.expires_at_ = now + allowed_time_window_;    }    return authsubsys_auth_denied; }