[comp.prog.boasting] Ход одного рефакторинга

Сейчас буду хвастаться собственными достижениями. Очень уж самому понравилось, как получилось. А, как известно, сам не похвалишь – никто не похвалит. В общем, весь нижеследующий текст с большим количеством примеров призван продемонстрировать, что программист должен быть в меру ленивым человеком – лень писать код должна диктовать его наиболее компактные формы :)

Итак, нужно было написать разбор конфига, один из тегов которого мог иметь следующие разрешенные варианты:

|| Вариант первый: используется только HTTP-аутентификация.
{auth_params "http_basic"
  {user_name "name"}
  {user_password "password"}
}

|| Вариант второй: HTTP-аутентификация + SSL с проверкой сервера.
{auth_params "http_ssl_server"
  {user_name "name"}
  {user_password "password"}
  {ca_file "path/to/ca/file"}
}

|| Вариант третий: SSL с проверкой и клиента, и сервера.
{auth_params "ssl_client_server"
  {ca_file "path/to/ca/file"}
  {client_cert_and_key_file "path/to/pem/file"}
  {client_private_key_password "password"}
}

Нужно было написать функцию, которая после завершения парсинга тега {auth_params} проверяла бы корректность сочетания вложенных тегов.

Первый вариант получался вот таким:

/* Вариант I */
void
tag_auth_cfg_t::on_finish(
  const cls_3::parser_context_info_t & context )
  {
    base_type_t::on_finish( context );

    const std::string & auth_type_name = base_type_t::query_value();
    auth_cfg_t::auth_type_t auth_type = get_auth_type( auth_type_name );

    // В режимах http_basic и http_and_ssl_server имя пользователя
    // и пароль обязательно должны быть определены. А в режиме
    // ssl_client_server -- не должны.
    if( autg_cfg_t::auth_ssl_client_server_check != auth_type )
      {
        if( !m_user_name.is_defined() )
          throw cls_3::semantic_ex_t(
              "mandatory tag should be defined: " +
              m_user_name.query_name() );
        if( !m_user_password.is_defined() )
          throw cls_3::semantic_ex_t(
              "mandatory tag should be defined: " +
              m_user_password.query_name() );
      }
    else
      {
        if( m_user_name.is_defined() )
          throw cls_3::semantic_ex_t(
              "tag should not be used: " +
              m_user_name.query_name() );
        if( m_user_password.is_defined() )
          throw cls_3::semantic_ex_t(
              "tag should not be used: " +
              m_user_password.query_name() );
      }
    ... // Дальнейшие проверки.
  }

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

/* Вариант II */
void
mandatory_tag_not_defined(
  const ... & tag, const std::string & auth_mode_name )
  { ... /* Порождение исключения с нужным сообщением. */ }

void
disabled_tag_not_defined(
  const ... & tag, const std::string & auth_mode_name )
  { ... /* Порождение исключения с нужным сообщением. */ }

void
tag_auth_cfg_t::on_finish(
  const cls_3::parser_context_info_t & context )
  {
    ... /* Начало точно такое же. */
    if( autg_cfg_t::auth_ssl_client_server_check != auth_type )
      {
        if( !m_user_name.is_defined() )
          mandatory_tag_not_defined( m_user_name, auth_type_name );
        if( !m_user_password.is_defined() )
          mandatory_tag_not_defined( m_user_password, auth_type_name );
      }
    else
      {
        if( m_user_name.is_defined() )
          disabled_tag_defined( m_user_name, auth_type_name );
        if( m_user_password.is_defined() )
          disabled_tag_defined( m_user_password, auth_type_name );
      }
    ... // Дальнейшие проверки.
  }

Уже лучше, но писать вызовы is_defined() и disabled_tag_defined() мне быстро надоело.  Во-первых, проверку наличия значения у тега можно переместить во вспомогательные функции. Во-вторых, зачем постоянно передавать имя режима аутентификации, если это можно сделать один раз? Стало получаться что-то вроде:

/* Вариант III */
class presence_checker_t
  {
  private :
    const std::string & m_auth_mode_name;
  public :
    presence_checker_t( const std::string & auth_mode_name )
      : m_auth_mode_name( auth_mode_name )
      {}
    void
    check_mandatory_tag( const ... & tag ) const
      { ... /* Проверка значения и порождение исключения. */ }
    void
    check_disabled_tag( const ... & tag ) const
      { ... /* Проверка значения и порождение исключения. */ }
  };

void
tag_auth_cfg_t::on_finish( const cls_3::parser_context_info_t & context )
  {
    base_type_t::on_finish( context );

    const std::string & auth_type_name = base_type_t::query_value();
    auth_cfg_t::auth_type_t auth_type = get_auth_type( auth_type_name );
    presence_checker_t checker( auth_type_name );

    // В режимах http_basic и http_and_ssl_server имя пользователя
    // и пароль обязательно должны быть определены. А в режиме
    // ssl_client_server -- не должны.
    if( autg_cfg_t::auth_ssl_client_server_check != auth_type )
      {
        checker.check_mandatory_tag( m_user_name );
        checker.check_mandatory_tag( m_user_password );
      }
    else
      {
        checker.check_disabled_tag( m_user_name );
        checker.check_disabled_tag( m_user_password );
      }

    // В режиме http_basic должны быть определены только два тега.
    if( auth_cfg_t::auth_http_only == auth_type )
      {
        checker.check_disabled_tag( m_ca_file );
        checker.check_disabled_tag( m_client_cert_and_key_file );
        checker.check_disabled_tag( m_client_private_key_password );
      }
    else
      {
        // В оставшихся режимах тег {ca_file} всегда должен быть определен.
        checker.check_mandatory_tag( m_ca_file );

        // Оставшиеся два тега должны быть определены только в
        // одном случае.
        if( auth_cfg_t::auth_ssl_client_server_check == auth_type )
          {
            checker.check_mandatory_tag( m_client_cert_and_key_file );
            checker.check_mandatory_tag( m_client_private_key_password );
          }
        else
          {
            checker.check_disabled_tag( m_client_cert_and_key_file );
            checker.check_disabled_tag( m_client_private_key_password );
          }
      }
  }

Этот вариант уже гораздо лучше. Хотя бы потому, что он позволяет обозреть весь метод on_finish – код понятен и легко проверяется.

Но все равно я вошел во вкус – мне казалось, что можно сделать еще короче:

/* Вариант IV */
class presence_checker_t
  {
  private :
    const std::string & m_auth_mode_name;
  public :
    presence_checker_t( const std::string & auth_mode_name )
      : m_auth_mode_name( auth_mode_name )
      {}
    void
    check( const ... & tag, bool is_mandatory )
      {
        // Если is_mandatory == true, то требует, чтобы тег
        // имел значение. Если is_mandatory == false, то тег
        // не должен иметь значения.
        ...
      }
  };

void
tag_auth_cfg_t::on_finish( const cls_3::parser_context_info_t & context )
  {
    base_type_t::on_finish( context );

    const std::string & auth_type_name = base_type_t::query_value();
    auth_cfg_t::auth_type_t auth_type = get_auth_type( auth_type_name );
    presence_checker_t checker( auth_type_name );

    // В режимах http_basic и http_and_ssl_server имя пользователя
    // и пароль обязательно должны быть определены. А в режиме
    // ssl_client_server -- не должны.
    const bool is_user_name_and_password_mandatory =
      (auth_cfg_t::auth_ssl_client_server_check != auth_type);
    checker.check( m_user_name, is_user_name_and_password_mandatory );
    checker.check( m_user_password, is_user_name_and_password_mandatory );

    // При работе по SSL тег {ca_file} обязательно должен быть задан.
    const bool is_ca_mandatory = (auth_cfg_t::auth_http_only != auth_type);
    // Параметры клиента должны быть определены только в режиме
    // ssl_client_server.
    const bool is_client_cert_mandatory =
      auth_cfg_t::auth_ssl_client_server_check == auth_type;

    checker.check( m_ca_file, is_ca_mandatory );
    checker.check( m_client_cert_and_key_file, is_client_cert_mandatory );
    checker.check( m_client_private_key_password, is_client_cert_mandatory );
  }

Вот этот вариант у меня и остался. Поскольку он намного компактнее всех предыдущих. Хотя и воспринимается чуть сложнее, чем предыдущий. Что не есть хорошо. Но дальше думать уже лень ;) Хотя я не сомневаюсь, что можно сделать еще проще.

PS. Кстати о количестве строк, производимых программистом. Вариант IV оказался самым компактным (всего на несколько строк короче варианта III и значительно короче варианта II). Вариант I я вообще не дописал, т.к. его размер превосходил все допустимые размеры. Однако, времени на написание окончательного варианта потребовалось раза в три больше, чем на работу над двумя первыми вариантами (грубо говоря: вариант II у меня получился через 20 минут после начала работы, а вариант IV – через час после начала). Что подтверждает мое старое наблюдение – чем спокойнее и неторопливее ход работы, тем меньше по объему получается код. Как следствие – чем спокойнее программист работает, тем меньше строк он пишет. Так что, смею предположить, у хороших программистов производительность в килостроках вообще никакая ;)