Здравствуйте, avpavlov, Вы писали:
D>>Это не валидация чего-то. Это не изменение логики программы. Это просто предохранитель, не более.
A>Хороший такой предохранитель, который кидает "AssertionError extends Error", который с 99% вероятностью пролетит мимо всех кэтчей и вырубит поток. Или у вас везде Throwable ловится?
Вы тоже читать не умеете? Что я писал выше по поводу того, когда работают ассершны? В продакшне, или во время девелопмента?
Здравствуйте, devcoach, Вы писали:
D>Здравствуйте, StanislavK, Вы писали:
>>Еще раз см. выше. То, что вы это повторяете, оно не станет правдой. assert — просто бесполезная и опасная штука, которую некоторые товарищи используют для якобы тестирования. Опасная она потому, что вместо того, чтобы писать надежный код с проверками, люди начинают городить ассерты и думают, что все будет пучком. D>Вы упорно не можете или не хотите услышать, что я пытаюсь до вас донести. Где я написал, что какие-то проверки нужно заменять ассертами? Если у вас в коде есть какая-то проверка, по итогам которой хотите бросить пользователю эксепшн, то, разумеется, ассерты использовать нельзя.
При чем тут пользователь? Ему плевать на ваши исключения. По поводу того, что проверки заменять ассертами — вы только об этом и пишете. И пример тоже об это. Например там есть проверка на то, что "пришел Вася, и зафигачил method3(), в котором забил на синхронизацию.", которую вы считаете почему-то не важной для продакшина, но достаточно важной для тестирования.
> В десятый раз повторяю: ассерты — это инструмент для тестирования, а не для управления реальной логикой приложения. Что такое тестирование вы понимаете? Следуя вашей выернутой наизнанку логике, я могу так же сказать: "А нафига нужны тесты вообще? Надо же просто проверку в коде, а потом эксепшн бросить!? Ведь тесты в продакшне не гоняются!" Ну конечно, они не гоняются. Их используют в процессе разработки, что бы отловить ошибки. И они точно так же, как и ассерты, не влияют на логику приложения в продакшне.
Для тестирования — тесты, а не ассерты. Эксепшн бросится точно так и в тесте и в продакшене, что просто замечательно.
>>Да это бред како-то. Если у вас есть гонка, то значит получится так, что данные будут в некорректном состоянии, что значит, что надо срочно падать или, хотябы, писать о проблеме в лог, а не довольствоваться выкинутым в релизе ассертом. D>См. выше.
Что смотреть? То, что вы игнорировать проблемы в продашн коде?
SK>>Вы не книжку пишете. Вне контекста, это просто пустые тезисы. Давайте ограничемся примерами того, где вам кажется, что этот иструмент работает и обсудим. Пример с многопоточностью пока выглядит не очень. D>Элементарнейший пример с многопоточностью. Есть какая-то заумная многопоточная логика. Есть некий метод, который можно вызывать только внутри критической секции. Если он вызван вне критической секции, то это косяк. То есть, на примере с synchronized, не сам этот метод synchronized, а он должен быть вызван внутри synchronized (надеюсь, идея понятна). Вот я написал два метода, method1() и method2(), все круто. А потом пришел Вася, и зафигачил method3(), в котором забил на синхронизацию. Не вопрос, мой ассершн сразу обрубит ему эту возможность:
Там должен быть не ассершн, а нормальный эксепшн, которы точно так же обрубит эту возможноть, причем не только в тестах.
D>Это не валидация чего-то. Это не изменение логики программы. Это просто предохранитель, не более.
"Предохранитель" для тестов. "Ха" два раза. Предохранители они не для тестов.
Здравствуйте, devcoach, Вы писали:
D>Здравствуйте, avpavlov, Вы писали:
D>>>Это не валидация чего-то. Это не изменение логики программы. Это просто предохранитель, не более.
A>>Хороший такой предохранитель, который кидает "AssertionError extends Error", который с 99% вероятностью пролетит мимо всех кэтчей и вырубит поток. Или у вас везде Throwable ловится? D>Вы тоже читать не умеете? Что я писал выше по поводу того, когда работают ассершны? В продакшне, или во время девелопмента?
Тогда возвращаемся туда, где СтаниславК писал тебе, что проверки должны быть всегда, потому что никакое тестирование не идёт в сравнение с богатством вариантов использования в продакшн
Здравствуйте, StanislavK, Вы писали:
SK>При чем тут пользователь? Ему плевать на ваши исключения. По поводу того, что проверки заменять ассертами — вы только об этом и пишете. И пример тоже об это. Например там есть проверка на то, что "пришел Вася, и зафигачил method3(), в котором забил на синхронизацию.", которую вы считаете почему-то не важной для продакшина, но достаточно важной для тестирования.
Два момента.
1) Почитайте на досуге JavaDoc к методу Thread.holdsLock(), а то позоритесь.
2) У вас какая-то извращенная логика касательно важности. Здесь речь идет не о том, что это "не важно для продакшна", а о том, что это до продакшна никогда не должно дойти в принципе. Что такое баг, знаете? Ну это когда программа работает некорректно. Релизить продукт с багами плохо, потому все стремятся минимизировать их количество. И когда баг находят, то есть два принципиально разных варианта зафиксить его: добавить каких-нибудь дополнительных проверок, что бы до места с багом выполнение не дошло, или же устранить причину бага. Как именно поступать — зависит от специфики конкретного участка кода. Если это публичный код, то обычно надо обрубить юзеру возможность делать запрещенные вещи. А если это ваш внутренний код, то надо устранить причину бага, а не проверки городить и эксепшны бросать, а потом обезопасить себя тестами. Так и вижу прям, как вы из кишков своего кода, когда не прошла валидация какой-нибудь вашей же внутренней переменной, бросаете юзеру эксепшн с общим смыслов "Не удалось выполнить операцию, так как наши разработчики из внутреннего метода А, про который вы ничего не знаете, передали в метод В, про который вы тоже ничего не знаете, некорретный аргумент C, про который вы также ничего не знаете." Пользователь, наверняка, обрадуется такому эксепшну.
SK>Для тестирования — тесты, а не ассерты. Эксепшн бросится точно так и в тесте и в продакшене, что просто замечательно.
Для тестирования и тесты и ассерты. Собственно, тест это обычно и есть набор ассертов, просто вынесенных в отдельный тестовый класс.
SK>Там должен быть не ассершн, а нормальный эксепшн, которы точно так же обрубит эту возможноть, причем не только в тестах.
Ну это опять таки к вопросу про JavaDoc к методу Thread.holdsLock().
Здравствуйте, devcoach, Вы писали:
D>Здравствуйте, avpavlov, Вы писали:
D>>>Это не валидация чего-то. Это не изменение логики программы. Это просто предохранитель, не более.
A>>Хороший такой предохранитель, который кидает "AssertionError extends Error", который с 99% вероятностью пролетит мимо всех кэтчей и вырубит поток. Или у вас везде Throwable ловится? D>Вы тоже читать не умеете? Что я писал выше по поводу того, когда работают ассершны? В продакшне, или во время девелопмента?
Здравствуйте, Hard_Club, Вы писали:
D>>Вы тоже читать не умеете? Что я писал выше по поводу того, когда работают ассершны? В продакшне, или во время девелопмента? H_C>А ExecutorService зачем тогда?
Это как связано?
Здравствуйте, avpavlov, Вы писали:
D>>Это не валидация чего-то. Это не изменение логики программы. Это просто предохранитель, не более.
A>Хороший такой предохранитель, который кидает "AssertionError extends Error", который с 99% вероятностью пролетит мимо всех кэтчей и вырубит поток. Или у вас везде Throwable ловится?
Здравствуйте, devcoach, Вы писали:
D>Здравствуйте, StanislavK, Вы писали:
SK>>При чем тут пользователь? Ему плевать на ваши исключения. По поводу того, что проверки заменять ассертами — вы только об этом и пишете. И пример тоже об это. Например там есть проверка на то, что "пришел Вася, и зафигачил method3(), в котором забил на синхронизацию.", которую вы считаете почему-то не важной для продакшина, но достаточно важной для тестирования. D>Два момента. D>1) Почитайте на досуге JavaDoc к методу Thread.holdsLock(), а то позоритесь.
Поясните, плз.
D>2) У вас какая-то извращенная логика касательно важности. Здесь речь идет не о том, что это "не важно для продакшна", а о том, что это до продакшна никогда не должно дойти в принципе. Что такое баг, знаете? Ну это когда программа работает некорректно. Релизить продукт с багами плохо, потому все стремятся минимизировать их количество. И когда баг находят, то есть два принципиально разных варианта зафиксить его: добавить каких-нибудь дополнительных проверок, что бы до места с багом выполнение не дошло, или же устранить причину бага. Как именно поступать — зависит от специфики конкретного участка кода. Если это публичный код, то обычно надо обрубить юзеру возможность делать запрещенные вещи. А если это ваш внутренний код, то надо устранить причину бага, а не проверки городить и эксепшны бросать, а потом обезопасить себя тестами. Так и вижу прям, как вы из кишков своего кода, когда не прошла валидация какой-нибудь вашей же внутренней переменной, бросаете юзеру эксепшн с общим смыслов "Не удалось выполнить операцию, так как наши разработчики из внутреннего метода А, про который вы ничего не знаете, передали в метод В, про который вы тоже ничего не знаете, некорретный аргумент C, про который вы также ничего не знаете." Пользователь, наверняка, обрадуется такому эксепшну.
Кто вам сказал, что пользователю надо показывать исключения? Если на то пошло, то пользователю ничего кроме "Извините мы упали" ничего показывать нельзя. Но это другой вопрос. Исключения надо в логи писать.
Проблема в том, что баги случаются на продакшине и глупо лишаться возможности их там заприметить. Если вы поставили ассерт, значит честно думаете, что в этом месте что-то может пойти не так. Но только почему-то думаете, что оно обязательно пойдет не так на тестах и вы все конечно же там все отловите. Извините, но так думать глупо.
Еще хороший вопрос — если что-то пойдет на продакжине не так, то как вы об этом узнате если уберете проверки?
SK>>Для тестирования — тесты, а не ассерты. Эксепшн бросится точно так и в тесте и в продакшене, что просто замечательно. D>Для тестирования и тесты и ассерты. Собственно, тест это обычно и есть набор ассертов, просто вынесенных в отдельный тестовый класс.
SK>>Там должен быть не ассершн, а нормальный эксепшн, которы точно так же обрубит эту возможноть, причем не только в тестах. D>Ну это опять таки к вопросу про JavaDoc к методу Thread.holdsLock().
Вам не стоит JavaDoc воспринемать настолько буквально. Слава богу там не было написано "убиться об стену".
Здравствуйте, avpavlov, Вы писали:
A>И? В Яве за историю существования столько ошибочных решений было, что нет смысла принимать javadoc за истину в последней инстанции.
Ну почитайте тогда JavaDoc к более "новому" методу ReentrantLock.isHeldByCurrentThread(), если вас Thread.holdsLock() не устраивает.
A>Метод Thread.holdsLock() — очевидный костыль, который в новом ConcurrencyAPI исправили добавлением методов Lock.tryLock/ReentrantLock.isLocked
Вы это на полном серьезе пишете, или стебетесь? Костыль к чему? Исправили что? Для чего вообще нужен ReentrantLock, и в чем его преимущества и недостатки перед synchronized знаете? Объясняю по пунктам:
1) Писать многопоточный код сложно.
2) Не менее сложно его тестировать.
3) ... поэтому разработчики Java любезно предоставили нам вспомогателные средства для дебага. Thread.holdsLock(), ReentrantLock.isHeldByCurrentThread() + куча подобных методов во всех синхронайзерах из пакета j.u.c..
4) Это не костыли к чему-либо, так как они ничего не исправляют и ни на что не влияют.
5) Как я уже написал выше, аналогом Thread.holdsLock() является ReentrantLock.isHeldByCurrentThread().
6) Метод же ReentrantLock.isLocked() так же является дебаговым, но показывает совершенно другое: что лок в принципе кем-то залокирован, а не текущим тредом, как это делает ReentrantLock.isHeldByCurrentThread(). То есть isLocked() не "исправляет" isHeldByCurrentThread(). Это вообще два разных метода, которые друг другу перпендикулярны.
7) Как можно "исправить" synchronized методом Lock.tryLock() я вообще не понимаю. tryLock() это дополнительная фича, а не "исправление". Более того, при большом желании аналогичный трюк можно проделать и с synchronized, см. Unsafe.tryMonitorEnter().
Здравствуйте, StanislavK, Вы писали:
SK>Проблема в том, что баги случаются на продакшине и глупо лишаться возможности их там заприметить.
Ну тогда. следуя вашей логике, тесты писать не надо. Ну а чо, тесты же никто в продакшне не запускает, лишаемся возможности отловить баг! Даешь проверки каждой доступной переменной после каждой строчки кода
SK>Если вы поставили ассерт, значит честно думаете, что в этом месте что-то может пойти не так. Но только почему-то думаете, что оно обязательно пойдет не так на тестах и вы все конечно же там все отловите. Извините, но так думать глупо.
Нет. Когда я ставлю assert, то я говорю не "здесь что-то может пойти не так", а "здесь всегда должно идти так". А если что-то "пошло не так", значит это баг, и мой продукт неработоспособен. А если он неработоспособен, то никакие проверки меня не спасут.
SK>Еще хороший вопрос — если что-то пойдет на продакжине не так, то как вы об этом узнате если уберете проверки?
Еще раз, я нигде не призывал менять что-то на assert. Я лишь говорил о том, что добавление асершнов позволяет улучшить качество кода. Не замена, а добавление.
мой любимый вопрос на собеседованиях,
правильного ответа правда не знаю, но "на вкус и цвет" мне тоже не очень нравится,
люди обычно говорят что это зависит от возможно/невозможно восстановиться после сбоя либо про какие-то внутренние ошибки, ни то ни другое тоже не очень
имхо это прежде всего про коммуникацию между программистами,
поэтому странно использовать checked между двумя своими классами (ибо код превращается черти во что без особой необходимости), а вот в библиотечных классах для коммуникации с (неизвестными) клиентами — нужно
D>Вы это на полном серьезе пишете, или стебетесь? Костыль к чему? Исправили что? Для чего вообще нужен ReentrantLock, и в чем его преимущества и недостатки перед synchronized знаете? Объясняю по пунктам:
До 1.4 ты не мог узнать залочен объект или нет — очевидный косяк (в том же .Net это было сразу). В 1.4 добавили костыль, а в 1.5 — уже нормальное АПИ, которое предоставляло полный контроль над поведением программы. Например, получать лок не дольше какого-то времени, и при неудаче переходить к другой ветке программы.
D>6) Метод же ReentrantLock.isLocked() так же является дебаговым, но показывает совершенно другое: что лок в принципе кем-то залокирован, а не текущим тредом, как это делает ReentrantLock.isHeldByCurrentThread(). То есть isLocked() не "исправляет" isHeldByCurrentThread(). Это вообще два разных метода, которые друг другу перпендикулярны.
Сам приписал мне высказывание ("То есть isLocked() не "исправляет" isHeldByCurrentThread()"), сам опроверг. Молодец.
D>7) Как можно "исправить" synchronized методом Lock.tryLock() я вообще не понимаю. tryLock() это дополнительная фича, а не "исправление". Более того, при большом желании аналогичный трюк можно проделать и с synchronized, см. Unsafe.tryMonitorEnter().
Не передёргивай, я не писал, что synchronized испорчен и требует исправлениф. "Испорчено" было АПИ, которое не давало возможности контролировать локи в полной мере.
D>Для чего вообще нужен ReentrantLock, и в чем его преимущества и недостатки перед synchronized знаете? Объясняю по пунктам:
Пункты-то когда будут? Всё что ты написал, к сравнению ReentrantLock и synchronized относится чуть менее чем никак
EE>поэтому странно использовать checked между двумя своими классами (ибо код превращается черти во что без особой необходимости), а вот в библиотечных классах для коммуникации с (неизвестными) клиентами — нужно
Хороши как средство борьбы с возвращаемыми null значениями, в расчёте, что кто-то где-то не забудет проверить на нулл и обработать как надо. Checked исключение очень эффективно тут.
Правда, в последнее время всякие findbugs-ы научились хорошо работать с @Nullable/@Nonnull и чекед исключения опять слегка стали ненужны.
Здравствуйте, devcoach, Вы писали:
D>Здравствуйте, StanislavK, Вы писали:
SK>>Проблема в том, что баги случаются на продакшине и глупо лишаться возможности их там заприметить. D>Ну тогда. следуя вашей логике, тесты писать не надо. Ну а чо, тесты же никто в продакшне не запускает, лишаемся возможности отловить баг! Даешь проверки каждой доступной переменной после каждой строчки кода
Почему это? По моей логике тесты писать надо, но они просто по-определению никогда не отловят все баги. Баги будут всегда и везде, даже в продакшине.
SK>>Если вы поставили ассерт, значит честно думаете, что в этом месте что-то может пойти не так. Но только почему-то думаете, что оно обязательно пойдет не так на тестах и вы все конечно же там все отловите. Извините, но так думать глупо. D>Нет. Когда я ставлю assert, то я говорю не "здесь что-то может пойти не так", а "здесь всегда должно идти так". А если что-то "пошло не так", значит это баг, и мой продукт неработоспособен. А если он неработоспособен, то никакие проверки меня не спасут.
Полностью не спасут, полностью ничего не спасет, но если будет нормальная проверка, она поможет идентифицировать проблему.
SK>>Еще хороший вопрос — если что-то пойдет на продакжине не так, то как вы об этом узнате если уберете проверки? D>Еще раз, я нигде не призывал менять что-то на assert. Я лишь говорил о том, что добавление асершнов позволяет улучшить качество кода. Не замена, а добавление.
Как так? Вы же сами предлагаете вместо:
if ( !Thread.holdsLock(this) ) {
throw new RuntimeException("Ops, method called without lock! We can't guarantee that application can continue to work properly");
}
писать:
assert Thread.holdsLock(this)
по-мне, так это просто выглядит этаким мазохизмом — знать, что тут можеть что-то пойти не так, но проверку выкинуть. Наверно, чтобы веселее было потом продакшин отлаживать.
Здравствуйте, avpavlov, Вы писали:
A>До 1.4 ты не мог узнать залочен объект или нет — очевидный косяк (в том же .Net это было сразу). В 1.4 добавили костыль, а в 1.5 — уже нормальное АПИ, которое предоставляло полный контроль над поведением программы. Например, получать лок не дольше какого-то времени, и при неудаче переходить к другой ветке программы.
Хорошо, но как это относится к чисто дебаговым методам ReentrantLock.isLocked(), ReentrantLock.isHeldByCurrentThread() и Thread.holdsLock()? До 1.5 конкарренси вообще не особо то работало в Java. volatile реордерились, j.u.c вообще отсутствовал. Но какое это имеет отношение к теме топика?
A>Не передёргивай, я не писал, что synchronized испорчен и требует исправлениф. "Испорчено" было АПИ, которое не давало возможности контролировать локи в полной мере.
Как именно оно было испорчено?
Здравствуйте, StanislavK, Вы писали:
SK>Почему это? По моей логике тесты писать надо, но они просто по-определению никогда не отловят все баги. Баги будут всегда и везде, даже в продакшине.
С этим никто и не спорит.
SK>Полностью не спасут, полностью ничего не спасет, но если будет нормальная проверка, она поможет идентифицировать проблему.
Повторюсь. Проблемы бывают разные. Есть проблемы "внешние", про которые надо сказать юзеру, что бы он принял на основе этого какое-то решение. Например, юзер передал некорректный аргумент, с которым ваша библиотека не будет работать. А есть проблемы "внутренние", которые сигнализируют о баге в коде. И в случае таких проблем, все уже не так очевидно.
SK>Как так? Вы же сами предлагаете вместо:
SK>
SK>if ( !Thread.holdsLock(this) ) {
SK> throw new RuntimeException("Ops, method called without lock! We can't guarantee that application can continue to work properly");
SK>}
SK>
SK>писать:
SK>
SK>assert Thread.holdsLock(this)
SK>
SK>по-мне, так это просто выглядит этаким мазохизмом — знать, что тут можеть что-то пойти не так, но проверку выкинуть. Наверно, чтобы веселее было потом продакшин отлаживать.
Э нее, я такого не предлагал. Я нигде не предлагал "вместо". Я лишь показал пример использования ассрешна, когда где-то в потрохах моего кода, в private методе стоит проверка что я использую свой же код корректно. Не юзер использует мой код, а я сам. Что мне даст этот эксепшн? Сообщит юзеру о том, что в системе баг? Сообщит юзеру, что мой продукт неработоспособен?
Здравствуйте, devcoach, Вы писали:
D>Здравствуйте, StanislavK, Вы писали:
SK>>Полностью не спасут, полностью ничего не спасет, но если будет нормальная проверка, она поможет идентифицировать проблему. D>Повторюсь. Проблемы бывают разные. Есть проблемы "внешние", про которые надо сказать юзеру, что бы он принял на основе этого какое-то решение. Например, юзер передал некорректный аргумент, с которым ваша библиотека не будет работать. А есть проблемы "внутренние", которые сигнализируют о баге в коде. И в случае таких проблем, все уже не так очевидно.
И что? О чем вообще это фраза? Вы хотите сказать, что "внутренние" ошибки надо игнорировать? И, вообще, при чем тут пользователь?
SK>>по-мне, так это просто выглядит этаким мазохизмом — знать, что тут можеть что-то пойти не так, но проверку выкинуть. Наверно, чтобы веселее было потом продакшин отлаживать. D>Э нее, я такого не предлагал. Я нигде не предлагал "вместо". Я лишь показал пример использования ассрешна, когда где-то в потрохах моего кода, в private методе стоит проверка что я использую свой же код корректно. Не юзер использует мой код, а я сам. Что мне даст этот эксепшн? Сообщит юзеру о том, что в системе баг? Сообщит юзеру, что мой продукт неработоспособен?
То, что это даст — во-первых не будет исполняться код, который уже не может гарантировать корректное исполнение, во-вотрых, не знаю как там у вас с этим, но у нормальных людей такие исключения будут сохранены в логе, по которому потом можно будет уже разобраться, что и где случилось.
Юзер же, кем бы он ни был, должен молучть отлуп, так как система в этой ситуации уже не способна выдать корректный ответ. Худшее, что можно сделать в это ситуации, это продолжать работать как ни в чем не бывало, т.к. система после этого будет в неопределенном состоянии.
Пользователь, наверняка, обрадуется такому эксепшну.
