OAK-11605 : removed usage of Guava's ImmutableSet.copyOf with LinkedSet#2184
OAK-11605 : removed usage of Guava's ImmutableSet.copyOf with LinkedSet#2184rishabhdaim merged 1 commit intotrunkfrom
Conversation
Commit-Check ✔️ |
| Set<String> filesToBeDeleted = | ||
| // Files present locally | ||
| ImmutableSet.copyOf(local.listAll()).stream() | ||
| SetUtils.toLinkedSet(local.listAll()).stream() |
There was a problem hiding this comment.
we are simply iterating over Set, so there is no need for immutability here.
There was a problem hiding this comment.
There is also no need to create a null-safe set as we do not do any look-up, so I think we can just remove the call to ImmutableSet.copyOf(). The set created by the stream is already immutable, so this call to Guava seems to be redundant.
There was a problem hiding this comment.
ImmutableSet.copyOf() is called to convert the array returned by local.listAll() to a collection, and then run stream operations on it.
It's either convert this to a collection OR use Arrays.stream(local.listAll()).
I don't have any strong opinion.
| Set<String> filesToBeDeleted = | ||
| // Files present locally | ||
| ImmutableSet.copyOf(local.listAll()).stream() | ||
| SetUtils.toLinkedSet(local.listAll()).stream() |
There was a problem hiding this comment.
There is also no need to create a null-safe set as we do not do any look-up, so I think we can just remove the call to ImmutableSet.copyOf(). The set created by the stream is already immutable, so this call to Guava seems to be redundant.
| fileNames = directoryBuilder.getChildNodeNames(); | ||
| } | ||
| Set<String> result = ImmutableSet.copyOf(fileNames); | ||
| Set<String> result = Collections.unmodifiableSet(SetUtils.toLinkedSet(fileNames)); |
There was a problem hiding this comment.
Also no need for immutability of null-safe lookups. This method is called only from the constructor, which just iterates over the contents of the set to add it to another set. I think this could be simplified. Directory listings may be long, so it may be worth to avoid copies here if possible.
There was a problem hiding this comment.
I would rather keep the optimizations separate from this ticket.
In this PR, I would try to keep the behavior as close as possible to the original.
3 participants
No description provided.