Hi Peter, Laurent,
Good work and I like the solution.
On 3/21/13 9:00 AM, Peter Levart wrote:
Hi Mandy,
JavaLogger and LoggerProxy are only used from within PlatformLogger.
Wouldn't it be better to declare them private? Their usage (one or the
other) depends on the 'loggingSupport' flag that is initialized with
PlatformLogger class. Usage of JavaLogger or LoggerProxy outside
PlatformLogger would be wrong, I think.
Yes JavaLogger and LoggerProxy classes can be made private.
Here's the new rev:
http://dl.dropbox.com/u/101777488/jdk8-tl/PlatformLogger/webrev.05/index.html
I removed the object() and forObject() methods and access the field
directly. I also renamed it to something more significant. forObject()
is not necessary, since it only had one call site and I just inlined
it. I also "took the opportunity". Now we're back to original + 9 LOC ...
I looked at it but not in great detail. In general it's very good clean
up. The comment in line 124-132 is good information and since the code
is evolving, I would suggest to take those count out.
valueOf is one common method name to find an instance of a given value.
Perhaps LevelEnum.forValue can be renamed to LevelEnum.valueOf(int)? It
seems to be useful to add a static method LevelEnum.getLevel(int) to
replace the calls to "LevelEnum.forValue(newLevel).julLevel".
The tests are in jdk/test/java/util/logging and
jdk/test/sun/util/logging. It'd be good to check if you want to extend
jdk/test/sun/util/logging/PlatformLoggerTest.java to cover all levels
(it's currently already checking several).
Thanks
Mandy
