среда, 7 сентября 2011 г.

Если не strcmp...

Опять встретилась проблема в коде. Не то, чтобы очень уж страшная проблема, некоторые даже думают что тем самым они наоборот вносят ясность в код. Но я, как обычно, против.

Давайте посмотрим на следующий код, который я для примера выдрал из ядра linux:
if (!strncmp(name, p, k) && p[k] == '=') {
 	p += k + 1;
	...
Вроде бы ничего особенного... Что же мне может не нравится в таком замечательном коде?

Глядя на этот код, лично я, никак не могу понять каким боком там стоит логическое отрицание? Проблема возникает, когда я пытаюсь этот код прочитать...

Если не strcmp... тогда кто?
Если strcmp ложно... а в чем собственно заключается истина?

Проблема в том, что результат функции не логический. Функция возвращает конкретную разницу, число. В спецификации написано открытым текстом, функция возвращает меньше, больше и равно нулю. Почему бы не написать просто: Если результат вызова strcmp равен нулю то...
if (strcmp(...) == 0) {
	...
Это не проблема работоспособности кода, это проблема понятности кода. Конечно все программисты знают что делает функция strcmp и что она возвращает. Но давайте представим на секунду, что мы видим совершенно незнакомую функцию: if (!foobar(...)) Как вы думаете, что проверяет это условие? Кто нибудь может сходу ответить?

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

Но, и самое главное, если правильно ее использовать в условии, можно сразу, не задумываясь ответить на вопрос - что же она возвращает. foobar(...) == '\0' - Эта функция возвращает char и вероятно наткнулась на символ окончания строки. foobar(...) == nullptr А эта возвращает нулевой указатель (Мы все знаем, что NULL в C++ - это плохо, а 0 очень похож на int :) ), Если же мы видим сравнение с нулем - это значит что нас интересует именно ноль сам по себе. Как число, как константа.

Конечно, никому в здравом уме не придет в голову писать if (foobar(...) == false)... Именно для этого случая существует логическое отрицание. И только в случае логического значения можно не писать явное сравнение.

По моему в давно пора перестать рассматривать ноль, как false, а не ноль, как true... Это было в прошлом веке.

Очень часто встречается, например, со стандартными библиотеками си:
int fd = open(...);
if (fd >= 0)
Ну какой ноль??? В мане серым по черному написано, что в случае ошибки функция возвращает -1, почему бы не написать условие явно?
if (fd == -1) {
	error;
Хотя, может быть это и не самый удачный пример (много этих стандартов в мире юникс, может быть где-то и не -1?).

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

А применяя вместо хитрых трюков типизированные константы, типа '\0', nullptr мы даем читателю подсказки к пониманию кода. С понятными именами у него не должно возникнуть необходимости к тому, чтобы полезть искать прототип, или хуже того - изучать реализацию.

6 коммент.:

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

Код должен читаться, буквально по английски.

Главное чтобы он код читался по С++-ски, а там !strcmp это идиома, и вообще std::string::operator == решает.

Андрей Валяев комментирует...

strcmp - это не логическая функция... просто с сишных времен повелось.

И речь не только про неё, речь условные выражения.

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

И сдесь тоже - в си все нули соответствуют false, следовательно через отрицание можно проверить на нуль. Можно, это не значит что так нужно делать. :)

В плюсах вообще лучше пользоваться string, это точно.

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

strcmp не логическая только в том смысле, что она возвращает не bool. А так она возвращает значение, преобразуемое в true, если строки отличаются, и в false, если не отличаются (В ISO С++: 4.12; а в С так и вовсе int работает логическим типом).

Сравнение с 0 ясности не добавляет, потому что имечко у strcmp не заточено под выражение намерений. Можно, конечно, написать == 0, или даже 0 == и тем намекнуть, что strcmp возвращает int, но это не значит, что нужно так делать. ;) Для strcmp это не имеет смысла, мало кто не знает, что возвращает strcmp. Для каких-нибудь нестандартных функций это может иметь смысл, но их лучше сразу делать возвращающими bool, или enum, или определять константы, или макро, на худой конец, с говорящим именем (для strcmp и это не выход). Не определили - увы, я считаю, можно писать что угодно.

Некоторые неленивые пишут хелперы вроде strequal(), только потом надо лезть проверять, а действительно ли этот хелпер работает так, как называется.

Вообще раз уж так повелось, чтобы писать !strcmp, то можно надеяться, что все об этом знают и код будет читаться.

А оптимизация не всегда бывает преждевременной и тем более не всегда такой остается )

Андрей Валяев комментирует...

Кстати далеко не все пишут !strcmp...
В том же линукс ядре присутствует и тот и другой варианты. Хотя булевый вариант лидирует.

С таким же успехом и указатели можно сравнивать через if (ptr); if (!ptr)... только это скрывает реальный тип переменной и может ввести в заблуждение.

Пусть strcmp всем известен, но лучше культивировать в себе хорошие привычки. :) Привычка работать с результатами в соответствии с их типом - хорошая ИМХО.

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

Если речь идет о выражении намерений, то на мой взгляд !strcmp выражает намерение сравнить строки на совпадение лучше, чем 0 == strcmp, потому что не отвлекает на другие варианты, кроме 0 и не-0. В то же время второй способ лучше выражает намерение определить отсутствие порядка в паре строк, если это когда-нибудь кому-нибудь понадобится.

Кстати, да, указатели. Здесь то же самое, если речь идет о выборе между NULL и не-NULL, то != NULL избыточно как == true, == NULL как == false, а чтобы предусматривать возможность сравнения указателя с чем-либо, кроме NULL, надо иметь веские основания. И сравнение с NULL все равно не полностью раскрывает тип, оно только намекает, что это указатель. Надо ли намекать, например, что, скажем, FILE * это указатель, если всего-то надо проверить, удалось ли открыть файл? Я думаю, не надо. Наоборот, надо намекнуть, что есть намерение использовать значение, возвращаемое fopen, не как описатель файла, а как признак успешной операции. Да это и лаконичнее.

А привычка, вернее отработанный и стиль, это само по себе хорошо. Я не думаю, что стоит ломать устойчивую невредную привычку ради новой, пусть даже новая вроде бы чем-то лучше.

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

@ Андрей Валяев

Верно, использование логических операторов с целочисленными операндами или указателями выглядит некрасиво. Понятно, почему это появилось - ранний С вообще был минимизирован по минимуму. Сейчас это вопрос привычки, но введение bool постепенно расставляет всё на свои места.

К тому же в С символ 0 имеет семантику гораздо шире, чем числовое значение нуля, почему, несмотря на то, что стандарт рекомендует использовать NULL, многие продолжают использовать 0 или логические операции типа if(!(f = fopen(...))) вместо правильного (с точки зрения стандарта) if( NULL != (f = fopen(...)) ).

@ Drum

На ваш взгляд !strcmp выражает сравнение на совпадение только по той причине, что фактически это уже одна неразрывная логическая конструкция, наподобие strequ, только записанная по другому.

В С int не работает логическим типом, также как и указатели или float. Условие в операторе if - это неявное сравнение с 0 (тело if выполняется, если выражение не равно 0).

В С++ проблемы могут возникнуть из-за того, что компилятор может выбрать функцию с аргументом int вместо перегруженной функции с типом bool, и для корректного выбора все равно придется писать 0 == strcmp.

С указателями ваша запись, как я упомянул выше, является потенциально непортируемой.