Hi Daniel,
This is not strictly necessary, but here are a couple of things you
might consider doing while changing this code:
Using new JDK 8 Map API, the following could be simplified:
580 static synchronized void add(Level l) {
581 purge();
582 // the mirroredLevel object is always added to the list
583 // before the custom Level instance
584 KnownLevel o = new KnownLevel(l);
585 List<KnownLevel> list = nameToLevels.get(l.name);
586 if (list == null) {
587 list = new ArrayList<>();
588 nameToLevels.put(l.name, list);
589 }
590 list.add(o);
591
592 list = intToLevels.get(l.value);
593 if (list == null) {
594 list = new ArrayList<>();
595 intToLevels.put(l.value, list);
596 }
597 list.add(o);
598 }
into...
static synchronized void add(Level l) {
purge();
KnownLevel o = new KnownLevel(l);
nameToLevels.computeIfAbsent(l.name, n -> new ArrayList<>()).add(o);
intToLevels.computeIfAbsent(l.value, v -> new ArrayList<>()).add(o);
}
Removing a KnownLevel could also remove ArrayList(s) when they become empty:
561 private void remove() {
562
Optional.ofNullable(nameToLevels.get(mirroredLevel.name)).ifPresent((x)
-> x.remove(this));
563
Optional.ofNullable(intToLevels.get(mirroredLevel.value)).ifPresent((x)
-> x.remove(this));
564 }
...
private void remove() {
nameToLevels.computeIfPresent(mirroredLevel.name, (n, l) ->
l.remove(this) && l.isEmpty() ? null : l);
intToLevels.computeIfPresent(mirroredLevel.value, (v, l) ->
l.remove(this) && l.isEmpty() ? null : l);
}
Regards, Peter
And if you implement Mandy's suggestion to return Optional<Level>
On 08/10/2016 06:21 PM, Mandy Chung wrote:
On Jul 29, 2016, at 4:54 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote:
http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.01/
This looks pretty good.
Since KnownLevel is now a Reference, I suggest to change KnownLevel::findByName,
findByValue and findByLocalizedLevelName to return Optional<Level> instead such
that the parse method implementaiton could be simplified.
Mandy
