вторник, 11 мая 2010 г.

[prog] Почему я предпочитаю один стиль оформления кода другому

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

Итак, какое-то время я писал следующим образом:

Образец #1
if( is_valid_trx_id( context_storage.get_current_context(), trx.id(),
   user_manager.find( trx.user_id() ) ) ) {

   try_enqueue_trx( trx );
   start_new_child_process_if_necessary();
}

Открывающая фигурная скобка на предыдущей строке (т.е. на строке, в которой находится if, switch, for, список параметров функции и т.д.) – это красиво и удобно. Но только на маленьких фрагментах кода. Вроде такого:

if( is_valid_trx_id( trx ) ) {
   try_enqueue_trx_id( trx );
   start_new_child_process_if_necessary();
}

В таких маленьких фрагментах хорошо видно, где именно находится открывающая фигурная скобка (и есть ли она вообще). Но, стоит только условному выражению внутри if-а расположиться на нескольких строках, да еще и содержать 4-5-6 закрывающих круглых скобок перед открывающей фигурной, как обнаружить месторасположение фигурной скобки оказывается уже сложнее.

Еще одна проблема с открывающей фигурной скобкой на предыдущей строке: если условие в if (for, while) занимает несколько строк, а затем еще и в составном операторе находится много кода, то бывает сложно выделить место начала составного оператора:

size_t items_viewed = 0;
for( awaiting_trx_container_t::const_iterator it = m_awaiting_trx.begin();
   it != m_awaiting_trx.end() && trx_to_process.size() < max_trx_at_once;
   ++it, ++items_viewed ) {
   log_current_trx( m_logger, m_action_description, *it );
   const trx_checking_result_t & checking_result = check_current_trx( *it );
   if( checking_result.is_processable_trx() ) {
      log_processable_trx( m_logger, m_action_description, *it );
      trx_to_process.push_back( *it );
      m_awaiting_trx.erase( *it );
   }
   // Здесь может быть еще несколько строк с какими-то действиями.
}

Чтобы устранить эту проблему, приходилось вставлять в самом начале составного оператора пустую строку, чтобы показать разделение между for-ом и его телом:

size_t items_viewed = 0;
for( awaiting_trx_container_t::const_iterator it = m_awaiting_trx.begin();
   it != m_awaiting_trx.end() && trx_to_process.size() < max_trx_at_once;
   ++it, ++items_viewed ) {

   log_current_trx( m_logger, m_action_description, *it );
   const trx_checking_result_t & checking_result = check_current_trx( *it );

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

Вот так я пришел к следующему стилю кодирования:

Образец #2
if( is_valid_trx_id( context_storage.get_current_context(), trx.id(),
   user_manager.find( trx.user_id() ) ) )
{
   try_enqueue_trx( trx );
   start_new_child_process_if_necessary();
}

Этот стиль лишен недостатков предыдущего. Но в нем есть проблемы, например, с “частоколом” из if-else-if или внутри switch-а:

if( can_process_trx( trx ) )
{
   // Какие-то действия.
}
else if( can_delay_trx( trx ) )
{
   // Еще какие-то действия.
}
else if( can_reroute_trx_to_another_host( trx ) )
{
   // Еще какие-то действия.
}
else if( can_throw_trx_away( trx ) )
{
   // Еще какие-то действия.
}
else
{
   // И еще какие-то действия.
}

Здесь в крайней левой позиции находится слишком много символов. Хочется видеть только if-else-if, а приходится еще и на фигурные скобки смотреть. В какой-то момент времени я понял, что мне это мешает, а вот GNU-тый стиль кодирования содержит нужное мне решение – отступ для составного оператора. С этим отступом “частокол” if-else-if становится более заметным, на мой взгляд:

if( can_process_trx( trx ) )
   {
      // Какие-то действия.
   }
else if( can_delay_trx( trx ) )
   {
      // Еще какие-то действия.
   }
else if( can_reroute_trx_to_another_host( trx ) )
   {
      // Еще какие-то действия.
   }
else if( can_throw_trx_away( trx ) )
   {
      // Еще какие-то действия.
   }
else
   {
      // И еще какие-то действия.
   }

Вот так я пришел к следующему стилю:

Образец #3
if( is_valid_trx_id( context_storage.get_current_context(), trx.id(),
   user_manager.find( trx.user_id() ) ) )
   {
      try_enqueue_trx( trx );
      start_new_child_process_if_necessary();
   }

Но и здесь оставалась одна закавыка: если действие под if-ом занимало одну строку, а само условие в if-e несколько строк, то было сложно различить где заканчивается условие и начинается действие:

if( is_valid_trx_id( context_storage.get_current_context(),
   trx.id(),
   user_manager.find( trx.user_id() ) ) )
   try_enqueue_trx( trx );

Поэтому при переносе длинных выражений на следующей строке я стал делать не один отступ табуляцией, а два. Т.о. продолжение выражение стало явно видно именно как продолжение:

if( is_valid_trx_id( context_storage.get_current_context(),
      trx.id(),
      user_manager.find( trx.user_id() ) ) )
   try_enqueue_trx( trx );

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

// В одну строку.
const ACE_Date_Time finish_at = minimum( trx.finish_at(), default_lifetime );
// В несколько строк. При переносах двойной отступ.
so_4::api::send_msg_safely(
      so_query_name(),
      "msg_next_turn",
      new msg_next_turn( current_context ),
      m_cfg.m_turn_size );

Вот так я и пришел к своему нынешнему стилю:

Образец #4
if( is_valid_trx_id(
      context_storage.get_current_context(),
      trx.id(),
      user_manager.find( trx.user_id() ) ) )
   {
      try_enqueue_trx( trx );
      start_new_child_process_if_necessary();
   }

Поскольку я пользовался в течении длительного времени (порядка нескольких лет) каждым из вышеперечисленных стилей, я могу читать/писать код в каждом из них. Без удовольствия в большинстве случаев, но и без проблем.

Ну и чтобы два раза не вставать, еще пару моментов.

Во-первых, я использую табуляцию для отступов вместо пробелов. Пришел к табуляции очень-очень давно. Году в 1992-м или 1993-м. Причем сначала причина была вообще очень примитивной и жестокой – C++ные исходники с табуляцией занимали раза в полтора меньше места, чем с пробелами. Во времена 1.2Mb 5-дюймовых дискет и медленных 286-х процессоров это было актуально. Потом добавился еще один бенефит – исходник с табуляциями было легко переформатировать для вставки в курсовые/дипломные работы. Не влезает исходник по ширине в A4 при размере табуляции в 4 пробела – ерунда, выставляем размер в 2 пробела и все помещается :) Кстати говоря, это до сих пор прекрасно работает при подготовке различного рода документации.

Во-вторых, я уже лет десять использую стиль, при котором тип возвращаемого функцией/методом значения записывается на отдельной строке. Это, на первый взгляд, дико выглядит для методов, которые возвращают int или std::string, но зато очень хорошо работает для более длинных имен типов:

class db_iface_t
   :  private ACE_Copy_Disabled
   {
   public :
      virtual ~db_iface_t();

      virtual void
      detect_known_and_unknown_packages(
         const package_id_list_t & full_id_list,
         package_id_list_t & unknown_ids,
         package_id_list_t & known_ids ) = 0;

      virtual std::auto_ptr< outgoing_message_list_t >
      select_outgoing_messages_for_sending(
         unsigned int message_count,
         const ACE_Date_Time & attempt_timestamp ) = 0;

      virtual std::auto_ptr< undelivered_SR_package_info_list_t >
      select_undelivered_SR_packages_for_resending(
         unsigned int package_count,
         const ACE_Date_Time & time_border ) = 0;

Disclaimer. Все вышеизложенное, вероятно, не очень актуально для работы с текстом программы исключительно за экраном компьютера. Умные редакторы, которые могут сами устанавливать высоту и размер скобочек, величину отступов и менять шрифты для разных элементов (как это делает, например, SourceInsight) + разные виды folding-а + различные браузеры исходных текстов – все это хорошо, но это только одна сторона медали. Вторая сторона – это распечатанные и/или включенные в документацию фрагменты кода. Мне с этим довольно часто приходится сталкиваться. Поэтому я придерживаюсь такого стиля кодирования, чтобы мой исходник можно было один-в-один вставить в качестве примера в документацию/блог и не думать о переформатировании.

4 комментария:

Alexander P. комментирует...

Чую мне ещё несколько лет двигаться от своего camelCase к not_camel_case. Наверное, тут сыграло наличие подчёркивания в моём первом нике - набило оскомину, видеть подчёркивание пока не хочу :) (К Ruby и Python это почему-то не относится - только к С/С++).
Кода я пока видел мало и не сильно обращаю внимание на его оформление. Хотя, думаю, сейчас бы GNU-тый стиль вывел бы меня из себя :).

А вообще, конечно, на свой код давности больше года смотреть уже страшновато (Например, наверное, никому не приходило в голову окружать пробелами `.` и `->` :)).

Отступление. Кстати, почему у Вас в последнем фрагменте кода в названии функции detect_known_and_unknown_packages идёт сначала known, потом unknown, а в параметрах, наоборот, сначала unknown, потом known? Никто не перепутает?

eao197 комментирует...

>Отступление. Кстати, почему у Вас в последнем фрагменте кода в названии функции detect_known_and_unknown_packages идёт сначала known, потом unknown, а в параметрах, наоборот, сначала unknown, потом known? Никто не перепутает?

Упс... Нужно будет подправить.

Вот яркий пример полезности code review :)

night beast комментирует...

Я правильно понял, что в if
тело ифа идет с отступом 2 таба?
или это только для ифов с большими условиями?

а в выражении for сколькими табами идет отступ?

не пробовал в больших выражениях переносить и закрывающую скобку:
if ( ....
.....
.....
) {
.....
} else {
.....
}

eao197 комментирует...

>Я правильно понял, что в if
тело ифа идет с отступом 2 таба?
или это только для ифов с большими условиями?


Только для if-ов с больщими условиями. Если условие if-а помещается в одну строку, то я пишу в одну строку.

>а в выражении for сколькими табами идет отступ?

Не совсем понял о чем речь. Если выражение внутри for(...) помещается в одну строку -- пишу в одну строку. Если не помещается, то при переносе части выражения отступ идет в две табуляции.

>не пробовал в больших выражениях переносить и закрывающую скобку:

не пробовал.