LINUX.ORG.RU

Краткая история о том, как не надо писать (и улучшать) код

 , ,


1

1

По мотивам разговора в Почему программы на с++ тормозят :) решил оформить это отдельной темой.

В исходниках GNU grep я наткнулся на такое:

/* Whether to output filenames.  1 means yes, 0 means no, and -1 means
   'grep -r PATTERN FILE' was used and it is not known yet whether
   FILE is a directory (which means yes) or not (which means no).  */
static int out_file;
  /* Which command-line options have been specified for filename output.
     -1 for -h, 1 for -H, 0 for neither.  */
  int filename_option = 0;
  /* Don't output file names if invoked as 'grep -r PATTERN NONDIRECTORY'.  */
  if (out_file < 0)
    out_file = !!S_ISDIR (st.st_mode);

Тут явно напрашивается enum вместо магических констант. О чем я сразу подумал. А дальше в коде я наткнулся еще на такой однострочник, с которым без поллитры не разобраться:

  out_file = (filename_option == 0 && num_operands <= 1
              ? - (directories == RECURSE_DIRECTORIES)
              : 0 <= filename_option);

В общем, я подумал, что эта лапша наверное родом откуда-то из конца 80-х - начала 90-х, когда код писали в «хакерских традициях». Но нет. Всё это безобразие было введено в 2019-м году в 3-х коммитах под следующими заголовками:

Автор сначала ввёл enum-ы как разумный разработчик. А потом выкинул их, заменив на числа и магические операции сравнения а ля больше/меньше/равно нулю.

Не делайте так. Не хакерствуйте над битами как для конкурса IOCCC.

★★

Последнее исправление: wandrien (всего исправлений: 2)

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

DumLemming ★★★
()

заходим на сайт автора первого коммита и читаем:

I have, at last, defended my dissertation (June 2018)

смотрим, кто это зааппрувил: чувак с университетским мылом.

Ну то есть налицо типичные академики без опыта коммерческой разработки. Чего еще от них ожидать-то?

Lrrr ★★★★★
()
Последнее исправление: Lrrr (всего исправлений: 1)
Ответ на: комментарий от Lrrr

ипичные академики

Не уж, я бы попросил.

Университет – заштатный, диссертация – болтологическая, ни одной серьезной работы нет.

Не тянет товарищ, совсем не тянет.

i586 ★★★★★
()
Ответ на: комментарий от wandrien

Код должен быть понятным без комментариев.

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

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

Было бы вообще идеально, если бы люди каменты читали. А начальство чтобы не требовало [при увольнении] «объяснить всё новичку», т.е. не пытались заставить меня делать всю ту же работу второй раз, демонстрируя и полнейшее неуважение к моему времени (вытекающее, полагаю, из копеечной зарплаты), и свой собственный кромешный идиотизм («всё» объяснить новичку невозможно в принципе, в любой проект надо долго входить).

В данной теме я в общем и целом за enum class BoolOptionValue { Disabled=-1, Default=0, Enabled=1 } (скрестил тебя и @firkax; да, я знаю что enum class это плюсявая фигня).

dimgel ★★★★★
()
Последнее исправление: dimgel (всего исправлений: 4)
Ответ на: комментарий от dimgel

Комменты могут быть как заголовки и подзаголовки, которые структурируют текст. Могут быть как отсылки к конкретным багам, RFC или требованиям ТЗ. Могут быть как лирические примечания. Как указания на применяемые алгоритмы. Да много чего можно а порою и нужно написать.

Но когда разработчик над объявлением переменной пишет коммент «минус 1 здесь означает вот этот ключ командной строки, а плюс один - вон тот», то это ошибка джуна, который еще просто кодить не научился.

wandrien ★★
() автор топика
Ответ на: комментарий от dimgel

enum class BoolOptionValue { Disabled=-1, Default=0, Enabled=1 }

Впрочем, фигню написал. По крайней мере с точки зрения читабельности: на плюсях лучше std::optional<bool> (2 байта), а если чисто enum, то Default надо брать вне диапазона допустимых значений: enum BoolOptionValue { Default = -1, False = 0, True = 1 }, тогда будет только один if: x == Default ? someDefaultValue : (bool)x. А разница между test edi,edi и cmp edi,-1 – 1 байт.

dimgel ★★★★★
()