Comments 22
List<UserEntity> getUsersForGroup(Long groupId) {
if (groupId == null) {
return Collections.emptyList();
}
return userRepository.findUsersByGroup(groupId);
}
когда ни одного пользователя не найдено?
В Spring Data запросы вида
@Query("select u from UserEntity u where u.group.id = :groupId")
List<UserEntity> findUsersByGroup(@Param("groupId") Long groupId);возвращают пустой список (ArrayList) для пустой выборки.
Так делает, на сколько я помню, не сам Spring Data, но и вообще любой ORM-маппинг, да и вообще чистый JDBC.
Про чистый JDBC не скажу, а Hibernate возвращает пустой список (ЕМНИП, пустой ArrayList).
Да, я как раз про это: все возвращают пустую коллекцию если запрос вернул 0 записей.
В том смысле что ответ на исходный вопрос пустая коллекция, но не потому что так делает Spring Data, а потому что так делают все и всегда и нет причин по которым кто-то будет отдавать что-то другое кроме пустой коллекции.
Возможно dskozin предположил что UserRepository#findUsersByGroup может возвращать null по аналогии с методами возвращающими одиночный объект. Но тут как раз все нормально: если мы не можем найти объект, то представлением пустого результата будет как раз null за неимением лучшего варианта. А для коллекций пустой result set запроса это пустая коллекция.
Если перевести все запросы возвращающие одиночный объект на Optional<T> то там тоже можно избавиться от null.
Вот это как раз умеет Spring Data.
Т.е. получается, что если меняется тип возвращаемого значения, то его нужно менять в двух местах…
Вы сейчас о чём?
Во всех вариантах метода getUsersForGroup тип (UserEntity) упоминается 1 раз — в джеренике на возвращаемом листе. Ну и ещё раз он, естественно присутствует в дженерике листа у UserRepository#findUsersByGroup.
Да, это два места. Но если мы меняем название класса сущности то, естественно, в обоих местах его и нужно будет сменить.
И я не понимаю как вопрос а что возвращает getUsersForGroup(Long) когда ни одного пользователя не найдено? превратился в если меняется тип возвращаемого значения, то его нужно менять в двух местах?
И вообще как-то не совсем синхронное ощущение от этого метода, поскольку есть и прямой возврат пустой коллекции и возврат чего-то, что возвращает репозиторий, а его возврат в свою очередь зависит от магии и т.п.
Где там магия?
- снаружи просят список пользователей для группы
- у нас (это проистекает из логики реализации) пользователь обязан иметь группу (то есть мы не интерпретируем группу
nullкакпользователи без группы) - Так что мы разделяем логику на два ветки, в зависимости от значения идентификатора группы:
null: пустая коллекция, так как мы сразу и заранее знаем что нет таких пользователей- не
null: запрос в БД, который всегда возвращает коллекцию
В результате метод всегда возвращает коллекцию и никогда null.
По факту на метод можно (и нужно, раз уж они упоминаются в статье) добавить аннотацию @Nonnull.
Опять же на счёт магии запроса к БД: нет там магии.
Мы выполняем запрос в БД вида (упрощённо) SELECT * FROM user WHERE group_id = ? и этот запрос, при любом значении параметра, любоая БД обработает одинаково и без магии — вернёт коллекцию строк подходящих под наше условие. И да, в некоторых случаях эта коллекция может быть пустой и это нормально.
Collection<Dto> dtos = getDtos();
Set<Integer> ids = dtos.stream().map(Dto::getId).collect(toSet());
Stream.of(1,2,3,4,5)
.filter(ids::contains)
.collect(toList());Я, может, чего не понял, но, вроде, задача — получить те id, что есть в списке (1,2,3,4,5). Зачем тогда собирать в Set id-шники из DTO (их же может быть 100500), когда можно взять Set.of(1,2,3,4,5) и бежать по списку DTO?
Set<Integer> search = Set.of(1,2,3,4,5);
dtos.stream()
.map(Dto::getId)
.filter(search::contains)
.distinct()
.collect(toList())
А вот ваш исходный
List<UserEntity> getUsersForGroup(Long groupId) {
Optional<Long> optional = Optional.ofNullable(groupId);
if (optional.isPresent()) {
return userRepository.findUsersByGroup(optional.get());
}
return Collections.emptyList();
}я бы преобразовал в
List<UserEntity> getUsersForGroup(Long groupId) {
return Optional
.ofNullable(groupId)
.map(userRepository::findUsersByGroup)
.orElseGet(Collections::emptyList);
}вместо вашего варианта:
List<UserEntity> getUsersForGroup(Long groupId) {
if (groupId == null) {
return Collections.emptyList();
}
return userRepository.findUsersByGroup(groupId);
}Тоже вариант, только в этом случае orElseGet не даёт преимущества перед orElse, т. к. Collections.emptyList() возвращает кэшированный список.
В случае с orElse метод Collections.emptyList() будет вызван в любом случае был ли у нас null на входе или нет, а в случае с orElseGet в него будет передан method reference который будет вызван только при необходимости.
Это алгоритмическая разница. Есть ли разница в производительности — я не измерял, и возможно orElse(Collections.emptyList()) будет даже эффективнее orElseGet(Collections::emptyList).
Но тут вопрос в том как будет в дальнейшем использоваться результат выполнения getUsersForGroup — ведь коллекцию, полученную из Collections.emptyList(), нельзя модифицировать. А если нам нужна такая возможность, то orElseGe(Collections::emptyList) можно заменить на orElseGet(ArrayList::new) и тут уже будет явный профит, по сравнению с orElse(new ArrayList())
По поводу дальнейшего использования, я тут посмотрел внутрь org.springframework.data.jpa.repository.support.SimpleJpaRepository, там есть такой метод
public List<T> findAll(Iterable<ID> ids) {
if (ids == null || !ids.iterator().hasNext()) {
return Collections.emptyList();
}
if (entityInformation.hasCompositeId()) {
List<T> results = new ArrayList<T>();
for (ID id : ids) {
results.add(findOne(id));
}
return results;
}
ByIdsSpecification<T> specification = new ByIdsSpecification<T>(entityInformation);
TypedQuery<T> query = getQuery(specification, (Sort) null);
return query.setParameter(specification.parameter, ids).getResultList();
}В текущем виде возможны два случая, когда возвращается пустой список: 1) на вход пришла пустая коллекция ключей и 2) запрос вернул пустую выборку.
Так вот в первом случае возвращается неизменяемый список, а во втором — ArrayList.
Если точнее, то реализация SimpleJpaRepository#findAll(java.lang.Iterable<ID>) возвращает разные типы коллекций, в зависимости от значения аргумента и структуры сущности:
null:java.util.Collections.EmptyList- пустая коллекция:
java.util.Collections.EmptyList - сущность имеет композитный ключ:
java.util.ArrayListиз всех результатовSimpleJpaRepository#findOne(ID) - сущность с некомпозитным ключом: то что вернёт ниже лежащий уровень, в случае с
Hibernateэто:
java.util.Collections.EmptyList: не совсем ясно когда это возможно, но вorg.hibernate.internal.SessionImpl#list(java.lang.String, org.hibernate.engine.spi.QueryParameters)есть такой флоу — не уверен на сколько он реалистиченjava.util.ArrayList: если дело дошло до выполнения запроса
Это можно представить в виде двух ответов на вопрос нужно ли реально выполнять запрос на БД:
нет:java.util.Collections.EmptyListи хватит с васда:java.util.ArrayList
и опять же — никакой магии.
А вот тут
try {
FileChannel sc = new FileInputStream(src).getChannel();
FileChannel dc = new FileOutputStream(new File(targetName)).getChannel();
sc.transferTo(0, sc.size(), dc);
dc.close();
sc.close();
} catch (IOException ex) {
log.error("ой", ex);
}, кроме всего уже описанного, можно не закрыть один и даже оба канала, что чревато всякими бяками.
И, в случае отсутствия библиотечных функций, для таких вещей, как минимум, необходимо использовать try-with-resources. Благо он появился уже очень давно.
А вот тут
static <T, Q extends Query> void setParams(
Map<String, Collection<T>> paramMap,
Set<String> notReplacedParams,
Q query) {
Set<String> params = paramMap.keySet();
notReplacedParams.stream()
.filter(params::contains)
.forEach(param -> query.setParameter(param, paramMap.get(param)));
}нет необходимости в отдельной переменной, так как вместо params::contains можно сделать сразу paramMap.keySet()::contains.
P.S.
А ещё этот тот самый void-метод меняющий состояние аргумента.
Мой улов за неделю