Please help me to figure out what is wrong with the method bellow and how can I solve it. The method takes a stream of Person object and Map with String value (a task name) as a key and an int value (a mark). The purpose of the method is to check whether a stream contains the particular tasks from allHistoryTasks variable and if does apply to this Map .putIfAbsentmethod(taskName, 0) to ensure that all the tasks are present in the Map (the purpose is to calculate an average mark later). When I run the test the UnsupportedOperationException error apears. When I comment the lines from the if statement and to forEach (lines 1, 2, 3, 4) test runs well. I'm new to Java and already spent several days on this issue but still can't solve it. Please tell me what is wrong here.
private Set<String> allHistoryTasks = Set.of("Phalanxing", "Shieldwalling", "Tercioing", "Wedging");
private String[] historyTasks = allHistoryTasks.toArray(new String[0]);
public Map<Person, Map<String, Integer>> addHistoryIfPresent(Stream<CourseResult> stream) {
return stream.collect(Collectors.toMap(
CourseResult::getPerson,
x -> {
if (allHistoryTasks.containsAll(x.getTaskResults().keySet())) //1
IntStream.range(0, allHistoryTasks.size()) //2
.parallel() //3
.forEach(i -> x.getTaskResults().putIfAbsent(historyTasks[i], 0)); //4
return x.getTaskResults();
}
));
}
custom classes & thread report
CodePudding user response:
The x -> {}
block is the 'value mapper'. It is supposed to turn an element of your stream into the value for a given map.
You have a stream of CourseResult
objects, and want a Map<Person, Map<String, Integer>>, so this function turns a
CourseResultobject into a
Map<String, Integer>`.
You do this by mutating things and that is a biiig nono. Your stream functions should not have any side-effects. Fortunately, the author of CourseResult
is way ahead of you and blocked you from making this error. You are calling .getTaskResults()
on your course result object and then trying to modify it. You can't do that, as the getTaskResults()
method returns a map that cannot be modified.
Presumably, you want to clone that map, and fix the clone. How do you do that? Well, you tell me, the API isn't clear. You could simply make a new ImmutableMap.builder()
, loop through whatever you want to loop through, and so on. From your code it's not quite clear what end map you do want.
Note also that you're using powers without knowing what you're doing - you have a parallel stream and are then forEaching through it, mutating the same variable, which you absolutely cannot do: This results in bugs where the result of an operation depends on an evil coin flip, in the sense that it can work fine today even if you rerun the tests a million times, and fail tomorrow. Separately, using parallel()
for such talks is borderline crazy - assuming the underlying stream impl actually parallelizes (.parallel()
is a hint, not a demand), it would just slow everything waaay down. allHistoryTasks
is tiny. This isn't what parallelism would be for.
CodePudding user response:
This might be the answer to your question. Set.of
method won't return the mutable set. So you need to declare a mutrable set like this to avoid this problem.
private Set<String> allHistoryTasks = new HashSet<>(Arrays.asList("Phalanxing", "Shieldwalling", "Tercioing", "Wedging"));
private String[] historyTasks = allHistoryTasks.toArray(new String[0]);
public Map<Person, Map<String, Integer>> addHistoryIfPresent(Stream<CourseResult> stream) {
return stream.collect(Collectors.toMap(
CourseResult::getPerson,
x -> {
if (allHistoryTasks.containsAll(x.getTaskResults().keySet())) //1
IntStream.range(0, allHistoryTasks.size()) //2
.parallel() //3
.forEach(i -> x.getTaskResults().putIfAbsent(historyTasks[i], 0)); //4
return x.getTaskResults();
}
));
}