On 08/16/2016 02:30 PM, Daniel Fuchs wrote:
Hi Peter,
On 16/08/16 13:05, Peter Levart wrote:
Hi Daniel,
Passing Stream<Level> returning extractors to Stream::flatMap is one way
of doing it. The other would be to make findByXXX methods return
Optional<KnownLevel> and then use Optional::map method on the result to
extract the needed property, so instead of:
Optional<Level> level = findByName(name, KnownLevel::referent);
You could simply make findByName return Optional<KnownLevel> and then:
Optional<Level> level = findByName(name).map(Reference::get);
Just plain null-when-absent-returning getters used with Optional::map
would do.
Well - not exactly because when looking for levelObject the KnownLevel
could become stale after it's returned. What we want is loop until
we find a levelObject which is not null (that's why the previous
version had some while loops in Level.parse).
Ah, I see. Right. Of course, my bad.
In that case, there is a possible race that could lead to exception here:
476 // add new Level.
477 Level levelObject = new Level(name, x);
478 return KnownLevel.findByValue(x,
KnownLevel::referent).get();
...between lines 477 and 478 and between lines 377 and 378, the newly
constructed Level object could already get GC-ed and so findByValue
could return an empty Optional.
You should do something like:
Level levelObject = new Level(name, x);
Level result = KnownLevel.findByValue(x, KnownLevel::referent).get();
Reference.reachabilityFence(levelObject);
return result;
Regards, Peter
But maybe I'm misunderstanding what you are saying?
best regards,
-- daniel
Regards, Peter
On 08/16/2016 12:42 PM, Daniel Fuchs wrote:
Hi Mandy,
I added an additional selector parameter to the find methods.
This made it possible to return Optional<Level> instead of
KnownLevel - and it does simply the parse() method.
http://cr.openjdk.java.net/~dfuchs/webrev_6543126/webrev.02
best regards,
-- daniel
On 11/08/16 20:12, Mandy Chung wrote:
On Aug 11, 2016, at 2:29 AM, Daniel Fuchs <daniel.fu...@oracle.com>
wrote:
On 10/08/16 17:21, 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.
We need to return KnownLevel because sometimes we need the
level object and sometimes the mirror.
So either findByName(String name, boolean mirror) or two methods:
findLevelByName and findMirroredLevelByName??
Or seriously consider to remove KnownLevel class by introducing a new
Level subclass with final Level.getName, Level.getLocalizedName,
Level.getResourceBundleName methods??
Mandy
