Skip to content
This repository has been archived by the owner on Sep 14, 2023. It is now read-only.

Добавить описание сравнения с null при помощи == #49

Open
daynin opened this issue Feb 9, 2017 · 10 comments

Comments

@daynin
Copy link
Contributor

daynin commented Feb 9, 2017

Если нам нужно проверять переменную на null или undefined, то лучше использовать двойное равно ==, так как одним сравнением мы убиваем двух зайцев

Править тут https://github.com/CSSSR/sputnik/blob/master/docs/JavaScript/Readme.md

Это нормально, так как:

null == null // true
null == undefined // true

null == '' // false
null == 0 // false
null == {} // false
null == [] // false
@daynin daynin changed the title Добавить описания сравнения с null при помощи == Добавить описание сравнения с null при помощи == Feb 9, 2017
@dzhiriki
Copy link
Contributor

dzhiriki commented Feb 9, 2017

В целом, да. Можно сделать замечание.

Но часто ли вообще нужно сравнение и с null, и с undefined? Семантическо это 2 разных значения. И чаще (на мой взгляд) нужно именно сравнение только с null.
А если в ходе встречаются и == null, и === null, то при беглом просмотре кода можно пропустить эту разницу и неправильно трактовать какой-то сценарий.
Как мне кажется, для сравнения и с null, и с undefined лучше использовать какой-нибудь R.isNil или _.isNil

@daynin
Copy link
Contributor Author

daynin commented Feb 9, 2017

@dzhiriki

лучше использовать какой-нибудь R.isNil или _.isNil

Зачем? Тянуть в код либу (конечно, она может уже и есть в code base) только ради проверки на null и undefined?

А если в ходе встречаются и == null, и === null

Именно поэтому лучше всегда использовать == null. Это гарантирует сравнение и с null и с undefined.

Но часто ли вообще нужно сравнение и с null, и с undefined?

Как показывает практика, это происходит крайне часто. Даже в проекте на TypeScript, где все довольно хорошо типизированно, буквально на прошлой неделе видел баг, связанный с выводом строки 1/undefined. Это может происходить, например из-за того, что API было плохо документировано или изменилось. На самом деле причин может быть много. Но так или иначе сравнивать переменную с null при помощи == просто эффективнее, здесь, как не странно, безопасность сравнения повышается, а не понижается.

А вот случай, где наоборот нужно было бы отдельно сравнивать с null и отдельно - с undefined - действительно довольно редкий, на мой взгляд, так как по сути, в обоих случаях мы хотим отсеять not a value case

@dzhiriki
Copy link
Contributor

dzhiriki commented Feb 9, 2017

Тянуть в код либу

Дело не в либе. А в том, чтобы делать сравнение сразу с null и undefined более явным.

Это гарантирует сравнение и с null и с undefined.

Я как раз про случай, когда в коде есть проверки только на null. И проверки и на то, и на то. Тогда при беглом осмотре кода запутаешься. А избавиться от всех проверок только на null не получится — они всё же бывают нужны. А вот изменить проверку сразу на null и undefined на отдельную функцию можно без проблем.

Как показывает практика, это происходит крайне часто.

У меня опыт несколько не такой. Строгих и не строгих проверок на null примерно одинаково и нестрогие как раз сделаны через R.isNil.

С Реактом еще нюанс в том, что null и undefined в пропсах вызывает разное поведение. undefined — вызовет дефолтное значение пропса, null — нет. С дефолтными параметрами в функциях аналогично.

@daynin
Copy link
Contributor Author

daynin commented Feb 9, 2017

@dzhiriki

Все верно, но это же уже не про сравнение. В случае с реактом и default arguments, это самое попадание в дефолтное поведение уже выступает в качестве одной из внутренних проверок. Но мы же никогда (возможно, есть редкие случаи) не хотим работать с null и undefined, и самый частый кейс именно сравнения c null - это, когда мы хотим отсеять не значения

Даже с учетом дефолтного поведения пропсов/аргументов, не вижу причин не использовать двойное равно. Что мы потеряем, если будем его использовать вместо тройного равно при сравнении с null?

@typeetfunc
Copy link

Даже в проекте на TypeScript, где все довольно хорошо типизированно, буквально на прошлой неделе видел баг, связанный с выводом строки 1/undefined

Это произошло потому что типы(спецификации) динамических данных не проверяли в динамике - не знаю кто вдруг решил что статические типы отменяют динамическую проверку. Статика это просто про то что можно проверить динамически в одном месте(месте получения данных) а дальше уже положится на стат анализ. Плюс есть то что в статике не проверишь - кастомные свойства.

Чтобы нормально проверять типы в динамике тебе всеравно нужна либа(это к вопросу про тянуть либу) - типа tcomb, runtypes etc

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

@daynin
Copy link
Contributor Author

daynin commented Feb 10, 2017

@typeetfunc

Это произошло потому что типы(спецификации) динамических данных не проверяли в динамике - не знаю кто вдруг решил что статические типы отменяют динамическую проверку.

Никто этого не решал. Так же как и типы не отменяют тесты, но суть разговора не в этом. Суть в том, что не нужно переусложнять то, что можно сделать просто.

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

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

Для проверки на null не нужна библиотека. Никто так же не отменял KISS. Библиотеку в проект следует тянуть, если она действительно полезна, случай же сравнения с null - не тот, чтобы тянуть что-либо в проект

@typeetfunc
Copy link

Ок вернемся к примеру.
Если вместо 1/undefined у тебя будет 1/[Object object] то станет лучше?
Еще недавно приводили смешной пример из апи IndexedDB:

var request = indexedDB.open("MyTestDatabase", 2.4);

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

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

Вообщем типичное для программирования лечение симптомов вместо причин.
В продакшене так делать норм - а вот учить людей так делать не оч.

Для проверки на null не нужна библиотека

Речь как раз про то что если все проверки спецификаций данных ограничиваются проверкой на нулл - то чтото пошло не так. Если данные проверяются - то либа с вероятностью 99% уже подключена.

Ну а про семантическую разницу Саша неплохо сказал уже.

@daynin
Copy link
Contributor Author

daynin commented Feb 10, 2017

@typeetfunc мне кажется, мы начинам сильно уходить в сторону.

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

Никто здесь не говорит, что все нужно проверить проверкой на null. Давайте не будем сейчас уходить в сторону верификации интерфейсов и прочего. Перед нами одна простая проблема - сравнение с null

Ну а про семантическую разницу Саша неплохо сказал уже.

Согласен, что семантическая разница определенно есть, с этим то никто и не спорит, но из-за этого сравнивать отдельно с null и отдельно с undefined некую сущность чтобы убедиться, что она является валидным значением - излишнее переусложнение на ровном месте

Допустим, у нас есть некоторые данные:

const user = {
  bio: {
    name: 'Vasya',
  },
}

У объекта user может быть так, что bio не указано, а именно быть undefined. Почему в этом случае не воспользоваться сравнением с null, чтобы отсеять сразу и undefined и null и вывести user.bio.name?

Да, наверное, @typeetfunc ты скажешь, что нужно проверять является ли bio объектом, а name - строкой. Ответ - не нужно. Если мы хотим это делать, то лучше использовать Flow/TypeScritp. Руками проверять каждое поле на соответствие определенному типу - значит неоправданно удорожать разработку. Но мы то сейчас говорим не о TypeScript/Flow/etc. А о правилах сравнения в js. Кстати, даже в этом смысле сравнение при помощи === - неверно, если уж копать глубже, так как не для примитивов сравнивать мы будем ссылки и писать:

/* Правильно */
if (a === b) {
  ...
}

Тогда тоже некорректно, потому что:

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

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

P.S.
Безусловно, возможны и случаи, когда нам нужно явно отличать null от undefined, но они довольно редки. Для этих случаев конечно же стоит использовать тройное равно, но наличие таких случаев не должно влиять на здравый смысл и заставлять всегда сравнивать с null отдельно, а с undefined отдельно.

@typeetfunc
Copy link

const user = {
  bio: {
    name: 'Vasya',
  },
}

то есть цель таки вывести имя? для этого нужны две вещи:

  1. функция безопасного доступа к вложенному свойству(типо R.path - если нет рамды то пишешь эту функцию сам - ровно один раз). Для абстракции чуть более сложных паттернов чем path есть Optional.
  2. тебе всеравно надо проверить что в name строка а не обьект или нулл иначе при выводе будет ерунда
    Итого в обоих примерах получилось что либо проверка на несуществование не нужна либо она делается гдето в библиотеке один раз за весь проект. Один раз за проект имхо можно и расписать.

значит неоправданно удорожать разработку

написание чего то типа t.object({ bio: t.object({ name: t.string }) }) непомерно удорожает разработку. Прям как propTypes.

Я согласен что речь вообще не про это. Но ты завел речь про то что проверка на пустоту это частый кейс и ее очень много в коде. И изза этого надо иметь возможно записывать эту проверку кратко при помощи двойного равно.

Я считаю что таком случае надо разобраться - а везде ли эти проверки семантически корректны или гдето это просто костыль потому что мы за данными не уследили? Там где они корректны их надо абстрагировать используя path и/или Optional(для 90% проектов хватит path). Там где где они не корректны с точки зрения семантики надо начать нормально проверять данные.
В бизнесс логике количество подобных проверок должно быть сведено практически к нулю.

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

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

@daynin
Copy link
Contributor Author

daynin commented Feb 10, 2017

то есть цель таки вывести имя? для этого нужны две вещи:

функция безопасного доступа к вложенному свойству(типо R.path - если нет рамды то пишешь эту функцию сам - ровно один раз). Для абстракции чуть более сложных паттернов чем path есть Optional.
тебе всеравно надо проверить что в name строка а не обьект или нулл иначе при выводе будет ерунда
Итого в обоих примерах получилось что либо проверка на несуществование не нужна либо она делается гдето в библиотеке один раз за весь проект. Один раз за проект имхо можно и расписать.

Тогда давайте напишем в правилах сравнения:

/* недопустимо */
if (a == b) {
  ...
}

/* недопустимо */
if (a === b) {
  ...
}

/* правильно */
if (R.equals(a, b)) {
 ...
}

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

Мне кажется, мы ушли вообще не туда. Изначально issue про то, что не нужно всегда сравнивать при помощи === и с null нужно сравнивать при помощи ==, так как это банально эффективнее. Если же нужно разграничивать null и undefined, то тогда стоит использовать и при сравнении с null тройное равно

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants