keith-turner commented on a change in pull request #1196: Update Accumulo to
build and run with Java 11
URL: https://github.com/apache/accumulo/pull/1196#discussion_r292992505
##########
File path:
core/src/main/java/org/apache/accumulo/core/client/summary/Summary.java
##########
@@ -117,13 +115,13 @@ public String toString() {
}
}
- private final ImmutableMap<String,Long> statistics;
+ private final Map<String,Long> statistics;
private final SummarizerConfiguration config;
private final FileStatistics fileStats;
public Summary(Map<String,Long> summary, SummarizerConfiguration config,
long totalFiles,
long filesMissingSummary, long filesWithExtra, long filesWithLarge, long
deletedFiles) {
- this.statistics = ImmutableMap.copyOf(summary);
+ this.statistics = Map.copyOf(summary);
Review comment:
I was reading about this new method and its not quite the same as
`ImmutableMap.copyOf()`. The key difference is the following line from the
javadoc.
```
If the given Map is an unmodifiable Map, calling copyOf will generally not
create a copy.
```
Therefore its still possible that what this returns could be mutated if I
did the following
* M1 = new HashMap
* M2 = Collections.unmodifiableMap(M1)
* M3 = Map.copyOf(M2)
* Mutate M1
Could see the result of mutating M1 in M3. That kinda sucks in my opinion.
I think it would be better to create our own utility method that guarantees
whats returned in actually immutable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
