DemetrNieA 13 окт 2015 в 22:29

Как написать красивый код и завалить проект

6 мин

40K

Java * Программирование * Качество кода *

Туториал

+12

Комментарии 28

Regis 14 окт 2015 в 00:48

В Guava на каждом методе c transform в JavaDoc написано, что transform создает View, а не новую коллекцию. В документации и примеры на это, и пояснения для чего так, и что делать если там нужна именно копия. Не читаем, осуждаем. ССЗБ? Да.

Второй пример — вообще смешно. Вы выделили жирным «then»часть предложения, но почему-то не «if» :)
Ваш случай подпадал под один вариант, а вы выбрали другой. И пишете, что документация плохая :)

PS; Вместо того, чтобы использовать коннекшн пул нулевого размера — может лучше его всё же полностью убрать? :)

PS2: Статью следовало бы назвать «Даже усли использовать хорошие библиотеки, вы всё равно можете выстрелить себе в ногу» :)

DemetrNieA 14 окт 2015 в 01:45

В Guava на каждом методе c transform в JavaDoc написано, что transform создает View, а не новую коллекцию. В документации и примеры на это, и пояснения для чего так, и что делать если там нужна именно копия.

Да, написано, но абсолютно неочевидно из сигнатуры метода, в результате чего такая ошибка мне встречалась крайне часто (причём и у опытных людей). Мораль в результате именно такая: не верь тому, как оно выглядит, обязательно понимай как работает.

Не читаем, осуждаем. ССЗБ? Да.

Читаем, обсуждаем, передаём опыт будущим поколениям.

Второй пример — вообще смешно. Вы выделили жирным «then»часть предложения, но почему-то не «if» :)

Потому что после then написан ещё пример, когда это условие верно — при связи между AppEngine и CloudSQL.

Ваш случай подпадал под один вариант, а вы выбрали другой. И пишете, что документация плохая :)

Почему Вы так считаете? Скорость открытия соединения в описываемом случае действительно мала, так что проект подпадал как раз под этот вариант.

PS; Вместо того, чтобы использовать коннекшн пул нулевого размера — может лучше его всё же полностью убрать? :)

Не могли бы Вы объяснить, что Вы имеете ввиду? Пулл нулевого размера — это и есть дефакто отсутствие пулинга. (если просто убрать настройку пулинга, то будет использоваться default value 1 и это как раз то, что делать совсем не стоит).

PS2: Статью следовало бы назвать «Даже усли использовать хорошие библиотеки, вы всё равно можете выстрелить себе в ногу» :)

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

Avers 14 окт 2015 в 09:02

Да, написано, но абсолютно неочевидно из сигнатуры метода

Получается «я книжку не читал, но она мне не понравилась». Если не читать документацию, а опираться на сигнатуры методов, то это уже не программирование/освоение библиотеки, а метод тыка в поисках «вроде бы работающего кода».

Regis 14 окт 2015 в 12:06

Не могли бы Вы объяснить, что Вы имеете ввиду? Пулл нулевого размера — это и есть дефакто отсутствие пулинга. (если просто убрать настройку пулинга, то будет использоваться default value 1 и это как раз то, что делать совсем не стоит).

Я подразумевал, что отключенный пуллинг и пул нулевого размера — не одно и тоже. Во втором случае остается оверхэд на оборачивание всех вспомогательных JDBC объектов в прокси пула.

Просмотрел бегло актуальные доки — судя по всему, со времен отделения c3p0 от самого Hibernate, hibernate.connection.pool_size со значением 0 именно отключает встроенный пул и вроде бы действительно отключает все дополнительные механизмы, нужные для пуллинга.

bohdan-shulha 14 окт 2015 в 18:16

> Потому что после then написан ещё пример, когда это условие верно — при связи между AppEngine и CloudSQL.

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

> The key words «MUST», «MUST NOT», «REQUIRED», «SHALL», «SHALL
> NOT», «SHOULD», «SHOULD NOT», «RECOMMENDED», «MAY», and
> «OPTIONAL» in this document are to be interpreted as described in
> RFC 2119.
> www.ietf.org/rfc/rfc2119.txt

DemetrNieA 14 окт 2015 в 18:56

Это инструкции для IETF. При написании документации к cloud sql им следовать, вообще говоря, не обязаны.

В данном случае имеет место рекомендация (и условие действительно выполняется).

bohdan-shulha 14 окт 2015 в 19:55

Вообще говоря, не обязаны, но в данном случае описание «may» более, чем подходит для выбранной цитаты. Даже дословный перевод это «может», а не «должен».

Рекомендации на то и рекомендации, что не обязательны к исполнению.

Я покидаю дискуссию, поскольку далее мы только будем толочь воду в ступе.

Спасибо.

P.S.: не поймите неправильно, я бы тоже ругался, что документация порекомендовала и рекомендация привела к неожиданным результатам, но на меня мои доводы подейстовали бы (как-то была похожая ситуция) :)

M_Muzafarov 14 окт 2015 в 05:36

Бейкап? это так называются битые бэкапы?

dyadyaSerezha 14 окт 2015 в 16:10

У автора просто любовь к «й»: прийдётся, бейкап… ;)

DemetrNieA 14 окт 2015 в 17:26

Хоть одно радует в таких комментариях — значит прочитали :).

Colwin 20 окт 2015 в 10:41

И запятые нужно ставить иногда :-)

vserykh 14 окт 2015 в 18:01

Запечёные, видимо :)

MaximChistov 14 окт 2015 в 06:18

Это же восьмая Java? тогда почему не так?

    @Override
    public ImmutableList<Entity> getCheckedEntityByIds(List<Integer> ids) {
        logger.info("Received " + ids.size() + " ids.");
        Collection<Entity> filteredEntities = ids.stream()
                .map(id -> storageService.lookupEntity(id))
                .filter(Entity::performCheck)
                .collect(Collectors.toCollection());

        notificationService.sendUpdate(filteredEntities.stream()
                .map(Entity::getId)
                .collect(Collectors.toCollection()));

        logger.info("Got " + filteredEntities.size() + " entities.");
        return ImmutableList.copyOf(filteredEntities);
    }

DemetrNieA 14 окт 2015 в 07:15

Потому что тогда бы этой истории не было бы — stream api действительно имеет более предсказуемое поведение, хотя и работает в среднем медленнее guava (в случае завершающего копирования).

Сам пример, очевидно, от версии Java слабо зависит. Писался на 8й для краткости исходного кода.

MaximChistov 14 окт 2015 в 09:10

stream api действительно имеет более предсказуемое поведение, хотя и работает в среднем медленнее guava (в случае завершающего копирования)

А можно пруфы? =/

tagir_valeev 20 окт 2015 в 10:59

Collectors.toCollection() без аргумента не бывает. Тогда уж Collectors.toList(). Но вообще стрим действительно продуманнее. Тут есть явная разница между коллекцией и потоком данных, одно с другим не спутаешь.

MaximChistov 20 окт 2015 в 19:45

мы только toList/toSet юзаем, писал на глазок :)

EngineerSpock 14 окт 2015 в 07:04

Если исключить то, что вы до конца не разобрались в исходных доках, то я бы обратил внимание на следующую мысль. Красивый код — не самоцель, он ещё и работать должен. Врёт не красивый код, а код, который, эм… собственно врёт.

mayorovp 14 окт 2015 в 07:27

Что-то я не понимаю. При автоматическом обрыве выполнения кода должны быть закрыты все используемые ресурсы, в том числе соединение с БД. Закрытие соединения с БД должно остановить все выполняемые запросы.

Если в этом механизме что-то поломалось, то при чем тут пулы соединений?

DemetrNieA 14 окт 2015 в 07:51

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

mayorovp 14 окт 2015 в 12:44

Что такое «обрыв выполнения»? Если это Thread.stop — то он возбуждает ошибку ThreadDeath, которая приводит, в том числе, к выполнению finally-блоков.

Если всюду где надо использовать примитивы вроде try-with-resources, и при этом не выполнять работы в конструкторах — то все запросы должны нормально отмениться.