понедельник, 23 сентября 2019 г.

[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;
}

Комментариев нет: