update sort parameter #41
Conversation
| new ArrayList<>( | ||
| sortFields.stream() | ||
| .filter(sortField -> mapping.containsKey(sortField.property())) | ||
| .map(sortField -> new Sort.Order( | ||
| sortField.direction(), | ||
| mapping.get(sortField.property()), | ||
| sortField.nullHandling() | ||
| )) | ||
| .toList() | ||
| ) |
There was a problem hiding this comment.
This adds additional overhead.
The previous code worked and returned a mutable ArrayList directly, but now we produce an immutable one and then copy it into a new mutable ArrayList.
We could simply remove the comment and create a test that modifies the returned sort to validate that it is mutable.
There was a problem hiding this comment.
Collectors.toList() explicitly says that
[...]
There are no guarantees on the type, mutability,
* serializability, or thread-safety of the {@code List} returned;
[...]
So this is the only way to make sure that it is mutable.
There was a problem hiding this comment.
I know about this line.
All the OpenJDK implementations return a mutable ArrayList.
There is no JDK in the wild that returns an immutable list.
This is why .toList() and .collect(Collectors.toUnmodifiableList()) exist.
| sortFields.stream() | ||
| .filter(sortField -> mapping.containsKey(sortField.property())) | ||
| .map(sortField -> new Sort.Order( | ||
| sortField.direction(), | ||
| mapping.get(sortField.property()), | ||
| sortField.nullHandling() | ||
| )) | ||
| // We do not use .toList() here as we potentially want to modify the sort list later in the StoreImpl | ||
| .collect(Collectors.toList()) | ||
| new ArrayList<>( | ||
| sortFields.stream() | ||
| .filter(sortField -> mapping.containsKey(sortField.property())) | ||
| .map(sortField -> new Sort.Order( | ||
| sortField.direction(), | ||
| mapping.get(sortField.property()), | ||
| sortField.nullHandling() | ||
| )) | ||
| .toList() | ||
| ) |
There was a problem hiding this comment.
I had Hide whitespace on, so the marked code did not include the previous code.
No description provided.