Почему драйвер Java MongoDB использует генератор условных чисел в условных выражениях?

206

Я видел следующий код в this commit для MongoDB Java Connection driver, и он появляется сначала быть какой-то шуткой. Что делает следующий код?

if (!((_ok) ? true : (Math.random() > 0.1))) {
    return res;
}

(EDIT: код был обновлен с публикации этого вопроса)

  • 13
    Какая часть вас смущает?
  • 0
    Прочитайте это stackoverflow.com/questions/10336899/…
Показать ещё 6 комментариев
Теги:
obfuscation

4 ответа

277
Лучший ответ

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

  • Эта строка безвозвратно запутана. Общий вид

    a? true : b

    для boolean a, b эквивалентно простому

    a || b

  • Окружающее отрицание и чрезмерные круглые скобки свертывают все дальше. Имея в виду законы Де Моргана, это тривиальное замечание, что этот фрагмент кода составляет

    if (!_ok && Math.random() <= 0.1)
  return res;

  • Признак, который изначально представил эту логику, имел

    if (_ok == true) {
  _logger.log( Level.WARNING , "Server seen down: " + _addr, e );
} else if (Math.random() < 0.1) {
  _logger.log( Level.WARNING , "Server seen down: " + _addr );
}

    — еще один пример некомпетентного кодирования, но обратите внимание на обратную логику: здесь событие регистрируется, если либо _ok, либо в 10% других случаев, тогда как код в 2. возвращает 10% времени и журналов 90 % времени. Таким образом, позднее совершение разрушило не только ясность, но и правильность.

    Я думаю, что в коде, который вы опубликовали, мы можем увидеть, как автор намеревался каким-то образом преобразовать оригинальный if-then в свое отрицание, требуемое для раннего условия return. Но затем он испортил и ввел эффективный "двойной отрицательный", изменив знак неравенства.

  • Проблемы с кодированием в сторону, стохастическая регистрация - довольно сомнительная практика сама по себе, тем более, что запись в журнале не документирует свое собственное своеобразное поведение. Очевидно, что целью является сокращение повторений одного и того же факта: сервер в настоящее время недоступен. Соответствующим решением является регистрация только изменений состояния сервера, а не каждого его наблюдения, не говоря уже о случайном выборе 10% таких наблюдений. Да, это требует немного больше усилий, поэтому давайте посмотрим некоторые.

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

Поделиться
  • 24
    Кроме того, насколько я могу судить, это официальный драйвер Java 10gen для MongoDB, поэтому, помимо мнения о драйвере Java, я думаю, что он дает мне мнение о коде MongoDB.
  • 5
    Отличный анализ всего нескольких строк кода, я мог бы просто превратить его в вопрос для интервью! Ваш четвертый пункт - реальный ключ, почему с этим проектом что-то принципиально не так (остальные могут быть отклонены как ошибки неудачного программиста).
Показать ещё 2 комментария
15

https://github.com/mongodb/mongo-java-driver/commit/d51b3648a8e1bf1a7b7886b7ceb343064c9e2225#commitcomment-3315694

11 часов назад от gareth-rees:

Предположительно, идея состоит в том, чтобы регистрировать только около 1/10 сбоев сервера (и, следовательно, избегать массового рассылки спама), не прибегая к расходам на поддержание счетчика или таймера. (Но верно ли сохранение таймера было бы доступно?)

Поделиться
  • 13
    Не для того, чтобы придираться, но: 1/10 времени он будет возвращать res, поэтому он будет регистрировать остальные 9/10 раз.
  • 22
    @Supericy Это точно не придирки. Это еще одно свидетельство ужасной практики кодирования этого человека.
7

Добавить член класса, инициализированный на отрицательный 1:

  private int logit = -1;

В блоке try выполните тест:

 if( !ok && (logit = (logit + 1 ) % 10)  == 0 ) { //log error

Это всегда регистрирует первую ошибку, затем каждую десятую последующую ошибку. Логические операторы "короткого замыкания", поэтому логит только увеличивается на фактическую ошибку.

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

Как было отмечено, это должно быть потокобезопасным:

private synchronized int getLogit() {
   return (logit = (logit + 1 ) % 10);
}

В блоке try выполните тест:

 if( !ok && getLogit() == 0 ) { //log error

Примечание. Я не думаю, что 90% ошибок - это хорошая идея.

Поделиться
  • 0
    Этот не потокобезопасный ...
  • 0
    Правда, это должно быть сделано потокобезопасным.
1

Я видел это раньше.

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

Поэтому иногда появлялись ранее невидимые новые "вопросы", и они появлялись в пакете, например, 100 из них подряд.

Программист был доволен тем, как работает программа, но он хотел каким-то образом улучшить программное обеспечение в будущем, по возможности открыли новые вопросы.

Таким образом, решение заключалось в регистрации неизвестных вопросов, но, как оказалось, было 1000 разных. Журналы стали слишком большими, и не было никакой возможности ускорить их, поскольку у них не было очевидных ответов. Но каждый раз время от времени появлялась партия вопросов, на которые можно было ответить.

Поскольку журналы становились слишком большими, и ведение журнала мешало регистрировать реальные важные вещи, которые он получил для этого решения:

Зарегистрировать только случайные 5%, это очистит журналы, в то время как в долгосрочной перспективе все еще покажет, какие вопросы/ответы могут быть добавлены.

Итак, если произошло неизвестное событие в случайном количестве этих случаев, оно будет записано в журнал.

Я думаю, что это похоже на то, что вы видите здесь.

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

Поделиться
  • 3
    За исключением того, что мы говорим о драйвере базы данных здесь ... неправильное пространство проблемы, IMO!
  • 0
    @ StevenSchlansker Я никогда не говорил, что это хорошая практика. Я удалил этот кусок кода и просто записал эти сообщения в другой файл.

Ещё вопросы

Сообщество Overcoder
Наверх
Меню