пятница, 16 декабря 2011 г.

[prog] Ввязался в спор о краткости имен идентификаторов

Вот в этот: Наименования переменных.

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

Когда я слышу разговоры о том, что идентификаторы (имена констант или переменных) должны быть короткими, я инстинктивно считаю, что люди предлагают вместо:

const size_t expected_bytes = calculate_expected_message_size();
const size_t bytes_read = load_data_from_stream( expected_bytes );
if( bytes_read != expected_bytes )
   ...;
binary_buffer_ptr_t whole_message = extract_raw_message();
binary_buffer_ptr_t message_payload =
   check_and_remove_protocol_headers( whole_message );

писать так:

const size_t e = calculate_expected_message_size();
const size_t r = load_data_from_stream( e );
if( r != e )
   ...;
binary_buffer_ptr_t m = extract_raw_message();
binary_buffer_ptr_t p = check_and_remove_protocol_headers( m );

Чего лично я не приемлю и за что “бью по рукам”. Ничего не поделать, учился у хороших учителей, давно, когда в образовании использовался Паскаль и книжки Вирта. А место, даже на дискетах, под исходные тексты экономить уже было не принято. Кстати, мониторы тогда были еще алфавитно-цифровые, с разрешением 80x25 знаков, так что текста на экране было намного меньше, чем сейчас. И IDE с автодополнением, подсказками и нафигацией по коду не было. Зато хороший стиль кода очень ценился.

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

Допустим, нам нужно сформировать SQL-ный select, где количество столбцов определяется внешними параметрами. Как это может выглядеть в императивном стиле с переменными? Как-то вот так:

std::string select_statement = "select id";
if( need_creation_time )
   select_statement += ", ctime";
if( need_modificaton_time )
   select_statement += ", mtime";
...
select_statement += " from my_table where ...";

Переменная одна, название ей дано длинное. А вот если мы напишем что-то подобное в функциональном стиле с иммутабельными сущностями, то получится что-то вроде:

s1 = "select id";
s2 = if( need_creation_time ) s1 + ", ctime" else s1;
s3 = if( need_modificaton_time ) s2 + ", mtime" else s2;
...
select_statement = sN + " from my_table where ...";

Т.е. если давать сущностям s1…sN какие-то вменяемые названия (вроде statement_with_opt_ctime, statement_with_opt_mtime и т.д.), то элементарно задолбешься выдумывать промежуточные названия. Да и смысла большого они нести не будут. Хотя, по моему мнению, обилие сущностей вида s1, s2 и т.д., не есть хорошо. Страшно далеко ФП от народа железа ;)

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

PS. Почему я так резко прореагировал на эту тему? Потому, что на днях случайно была найдена серьезная ошибка. В одной из версий программы один разработчик заменил код:

stream << range.m_left
   << range.m_right
   << range.m_client
   << range.m_priority;

на код:

// Если диапазон оказался слишком большим, то нужно его сузить
// принудительно передвинув правую границу.
id_t right = range.m_right;
if( range.m_right > range.m_left + package_size )
   right = range.m_left + package_size;
stream << range.m_left
   << right
   << range.m_client
   << range.m_priority;

Второй разработчик должен был эти же изменения перенести (смержить средствами системы контроля версий) в параллельно развиваемую ветку программы. Полагаю, опытные разработчики уже догадались, что у него получилось:

// Если диапазон оказался слишком большим, то нужно его сузить
// принудительно передвинув правую границу.
id_t right = range.m_right;
if( range.m_right > range.m_left + package_size )
   right = range.m_left + package_size;
stream << range.m_left
   << range.m_right
   << range.m_client
   << range.m_priority;

Понятное дело, что ошибка обнаружилась далеко не сразу и случайно. А уж если бы вместо range, right, m_left, m_right использовались бы имена вида R, L, R1, R2 и т.д., то я бы только функциональщикам и пожелал бы сопровождать такой код.

PPS. Еще ссылки:
- заметка в блоге lionet на какое-то исследование понятности написанного в разном стиле кода;
- очень интересная реакция на это исследование в блоге sleepy_drago;
- скептическая точка зрения Бертранда Мейера на околопрограммистские исследования (на английском).

29 комментариев:

  1. Ну функциональшики прибегут и скажут, что вместо

    id_t right = range.m_right;
    if( range.m_right > range.m_left + package_size )
    right = range.m_left + package_size;

    напишут в нужном месте:

    << std::min(range.m_right, range.m_left + package_size)

    Наверное:).

    Для SQL может напишут так:
    let query = "select id" append_if (need_creation_time, ", ctime") append_if (need_modificaton_time, ", mtime")
    append " from my_table where...";

    Это не знаю какой язык, но на Haskell думаю такое делается с точностью плюс-минус детали синтаксиса:). На С++, кстати, тоже.

    Ну это я со своей С++-колонны смотрю.

    ОтветитьУдалить
  2. 1. вирта к позорному столбу!!!

    емнип "алгоритмы + структуры данных = программы" полны идентификаторов вида e1, e2

    ты, наверно, помнишь, что я любитель инлайнить (не для производительности, а для понятности), но как он там инлайнит -- охренеть (впрочем, возможно, это общая беда не-ооп языков)

    ОтветитьУдалить
  3. @Alexander P.

    > std::min(range.m_right, range.m_left + package_size)

    Можно и так, но тогда промежуточное логирование и отладочные печати делать сложнее ;)

    ОтветитьУдалить
  4. @имя:

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

    В бытность студентом кафедры "Вычислительной математики и программирования" довелось попрограммировать разные формулы и методы. Там однобуквенные имена в порядке вещей. Никого не парят и читаются нормально. Может потому, что там намного важнее узнаваемость исходных математических конструкций.

    ОтветитьУдалить
  5. 2. код, похоже, ужасный (хотя может есть оправдания, не знаю)


    А. уж если патчить, то весь range, в смысле примерно так:

    range_type new_range = range;
    new_range.m_right = std::min(range.m_right, range.m_left + package_size);

    и если можно, то пропатчить range, а не вводить новый

    В. возможно, что вывод портянки stream << range.m_left
    << range.m_right
    << range.m_client
    << range.m_priority; надо заменить на stream << range;

    хотя надо смотреть, юзается ли эта последовательность (и может ли юзаться) где-то в другом месте -- я считаю, что инлайн такой функции, если она не юзается где-то еще, нормален

    ОтветитьУдалить
  6. да, в формулах однобуквенные имена в порядке вещей, но там вирт рассказывал всякие вещи типа сортировки слиянием, пирамидальной сортировки и т.д.

    ОтветитьУдалить
  7. если уж однобуквенные имена...

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

    скажем, e от expected -- ужасно, а вот например l или el от expected length -- приемлемо

    ОтветитьУдалить
  8. @имя:

    >range_type new_range = range;
    new_range.m_right = std::min(range.m_right, range.m_left + package_size);


    В итоге там было сделано как-то так:

    range_t range = detect_actual_range(original_range, *m_db);

    где выспомогательная функция detect_actual_range выполняла коррекцию не только правой, но и левой границы диапазона.

    Кстати говоря, я привел не полный пример. Там было два if-а перед коррекцией значения right -- проверялся еще и тип диапазона, для некоторых типов такая коррекция была запрещена.

    ОтветитьУдалить
  9. @имя:

    >хотя надо смотреть, юзается ли эта последовательность (и может ли юзаться) где-то в другом месте

    Во-первых, больше не юзается.
    Во-вторых, это часть связывания prepared statement-а со значениями параметров из этого statement-а. В таком виде получается намного понятнее, чем упрятывание этой операции в отдельную функци (оператор сдвига для всего range).

    ОтветитьУдалить
  10. Этот комментарий был удален автором.

    ОтветитьУдалить
  11. то, как я пишу такого рода вещи, в переводе на с++ будет выглядеть так:

    std::string query = string("select id")
    + (need_creation_time ? ", ctime" : "")
    + (need_modificaton_time ? ", mtime" : "")
    + " from my_table where..."

    с разбивкой на строчки на "+"

    ОтветитьУдалить
  12. @имя
    >то, как я пишу такого рода вещи, в переводе на с++ будет выглядеть так:

    А как это выглядит без перевода на С++?

    ОтветитьУдалить
  13. > А как это выглядит без перевода на С++?

    примерно так же :-) в базу я обычно лезу из перла или пхп

    ОтветитьУдалить
  14. @Евгений Охотников

    насчет 2В ты меня убедил

    а насчет 2А? там можно было менять range?

    range.m_right = std::min(range.m_right, range.m_left + package_size)

    ОтветитьУдалить
  15. @имя:

    >там можно было менять range?

    range туда передавался как константная ссылка. Просто так ее изменить нельзя.

    Можно было бы сделать либо так:

    range_t actual_range = original_range;
    if( is_adjustable_range(original_range) ) actual_range.m_right = std::min(...);

    Либо же обойтись без копирования всего range, как изначально и было сделано. range?

    ОтветитьУдалить
  16. @Евгений Охотников

    3. то что получилось

    range_t range = detect_actual_range(original_range, *m_db);

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

    some_function( a, b, actual_range(range, *m_db), c, d);

    4. кстати -- плохо, что с++ не дает возможность запретить видимость переменной

    5. префикс m_ это ужасная вещь, хотя почему-то получил распостранение

    6. в оригинальном коде наличие переменных с одинаковыми (с точностью до m_) названиями right и range.m_right это тревожный звонок

    я, хотя и использую паттерн

    Point(int x, int y): x(x), y(y) {};

    но нарывался на баги, если чуть уклонялся от этого

    щас детали не вспомню, но вроде бы типа такого:

    Point(int x, int y): x(x-1), y(f(x,y)) {};

    использовать Point(int x_, int y_): x(x_), y(y_) {}; не хочется по эстетическим соображениям

    ОтветитьУдалить
  17. 7. кстати, и в математике можно юзать двухбуквенные идентификаторы:

    l с индексом e это expected length

    l с индексом r это real length

    поскольку там все неформально, то даже наличие переменных r=2 и e=1 этому не помешает, надо только объяснить, что это не l2 и не l1

    ОтветитьУдалить
  18. и еще на тему моего вопроса 2А: менять переменную на ходу это не всегда лучший вариант

    т.е. когда видно, что тут мы постоянно заняты мутацией (полей) какой-то переменной, то да, нормально

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

    ОтветитьУдалить
  19. наконец, беря из рассказа thesz:

    1. короткие идентификаторы

    2. Наш пример "недавнего студента на Хаскеле".

    Две недели на чтение мануалов и разговор о том, что к чему. Ещё месяц на въезжание в представление предметной области (и расширение библиотеки под новые задачи). После чего мы говорили только о предметной области.

    Про исходники - это жёсткий Хаскель с GADT, арифметикой и прочими вычислениями на уровне типов


    в результате все очень похоже на то, что thesz выдает желаемое за действительное :-)

    ОтветитьУдалить
  20. Думаю, что это вы недооцениваете значимость понятности программ, которые пишутся не только (да и не столько) для выполнения компьютером, сколько для передачи намерений программиста будующим разработчикам программы.

    полностью согласен

    и насчет коротких идентификаторов -- если они используются один раз, то лучше их заинлайнить или удлинить

    единственным оправданием может быть их использование много раз, типа

    int y=f(...);
    int ...=g(y+1,y-2,y*y);

    ОтветитьУдалить
  21. @имя:

    >и насчет коротких идентификаторов -- если они используются один раз, то лучше их заинлайнить или удлинить

    единственным оправданием может быть их использование много раз, типа


    Угу. Я тоже не понял, откуда в программе может появиться много сущностей, которые используются всего один раз. ИМХО, если какая-нибудь константа повторяется несколько раз в одном выражении (вроде твоего примера с g(y+1,y-2,y*y)), то это уже многократное использование.

    К сожалению, thesz не дал в свое заметке примеров кода для иллюстрации своей мысли. Поэтому я нахожусь под впечатлением, что не правильно его понял.

    PS. Обсуждение своего кода не продолжаю, т.к. без публикации всей функции и рассказа ряда особенностей это делать бессмысленно. А опубликовать код целиком не могу.

    PPS. По поводу префиксов и других особенностей нотации нужно будет заводить отдельную тему.

    ОтветитьУдалить
  22. @Евгений Охотников

    Обсуждение своего кода не продолжаю

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

    хотя конечно пользы было бы еще больше, если бы ты, или кто-то, занялся критикой моих высказываний;

    предположильно, тут у тебя все ведут себя слишком политкорректно -- мне больше нравится как на лоре, где каждый может выступить и раскритиковать

    По поводу префиксов и других особенностей нотации нужно будет заводить отдельную тему.

    ок, жду

    ОтветитьУдалить
  23. мой пост, начинающийся словами "и еще на тему моего вопроса 2А:" это фактически спор с самим собой -- и гораздо лучше, если бы мне возразил кто-то другой

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

    ОтветитьУдалить
  24. кстати, насчет вирта: "алгоритмы + структуры данных = программы" доступны он-лайн http://www.ethoberon.ethz.ch/books.html

    правда там доступна улучшенная версия (с обероном), версия, которую я читал в советском переводе это 1976 год -- там еще хуже

    впрочем, на странице 70 можно видеть имена r0, r1, r2, которые вменяемый человек назвал бы source1, source2, destination либо src_ride_1, src_ride_2, dst_ride

    еще раз -- к позорному столбу вирта, или вот так:

    Whereas Europeans generally pronounce his name the right way ('Nick-louse Veert'), Americans invariably mangle it into 'Nickel's Worth.' This is to say that Europeans call him by name, but Americans call him by value.

    ОтветитьУдалить
  25. s/ride/rider/

    впрочем, копирование там идет в 2 стороны, так что src и dst плохо подходят, возможно small_rider_1, small_rider_2, big_rider лучше

    ОтветитьУдалить
  26. ну и наконец -- если читать это как книгу по алгоритмам, и не читать монстров, которые он пишет -- вполне приятная книжка

    ОтветитьУдалить
  27. @имя:

    >ок, жду

    Как-то так: http://eao197.blogspot.com/2011/12/progmemories-camelcase-lowercase.html

    ОтветитьУдалить
  28. По моему Сергей прав, у меня тоже в C++ и в питоне длинные идентификаторы, коротких кроме идеоматичных i, j и т. п. практически нет, но в ocaml количество коротких на порядок больше, но они практически все сильно локальные, область действия чаще всего одна или несколько строк. Причин основных две.

    Первая паттерн матчинг типичная конструкция которого это:

    | [Конструктор] [переменная] -> [выражение]

    обычно все это на одной строке определение прямо перед глазами, смысла в длинном идентификаторе переменной никакого.

    Вторая очень часто используется комбинирование функций, например в виде потока:

    let is_prime x =
    2 -- x
    |> take_while (fun i -> i*i <= x)
    |> for_all (fun i -> (x mod i) != 0)

    то есть типично
    [функция] |> [функция] [короткая лямбда] |> ...

    Здесь также выражение с короткими идентификаторами всегда перед глазами. И эти короткие идентификаторы читабильность только улучшают.

    ОтветитьУдалить
  29. @Rustam:

    Не знаю, если речь идет о математических выражениях, тогда короткие идентификаторы в тему. Я вот больше такой код пишу -- http://codepad.org/dJCnqIHV -- так мне ничего сокращать не хочется.

    ОтветитьУдалить