On 14/09/2018 20:16, Peter Levart wrote:
Hi Alan,
Just a few comments about implementation.
In Reflection:
292 boolean shouldSkip = false;
293 for (String filteredName : filteredNames) {
294 if (member.getName() == filteredName) {
295 shouldSkip = true;
296 break;
297 }
298 }
...now that filteredNames is a Set<String>, you could simplify this
loop to:
shouldSkip = filteredNames.contains(member.getName());
I was trying to keep the changes to the filtering implementation as
minimal as possible but you are right that replace the array with a set
means the loop can be replaced (thanks!).
:
I don't know if it's better, but you could use just the immutable Map
without HashMap. Instead of:
262 map = new HashMap<>(map);
263 map.put(containingClass, Set.copyOf(names));
264 return map;
you could do this:
@SuppressWarnings("unchecked")
Map.Entry<Class<?>, Set<String>>[] entries =
map.entrySet().toArray(new Map.Entry[map.size() + 1]);
entries[entries.length - 1] = Map.entry(containingClass,
Set.copyOf(names));
return Map.ofEntries(entries);
This should work because the registerFilter method is called at most
once per map/containingClass.
The existing code is easy to read. Using an immutable map has a few
advantages but using map entries is easy to get wrong. I think I'll
leave it as is for now. There are a few additional classes that should
also be added to the filter in another round so that might be the time
to re-visit it.
I've put a new webrev here, the only change is the replacement of the
old loop to use contains as you suggested:
http://cr.openjdk.java.net/~alanb/8210496/2/webrev/index.html
-Alan
