Alan, David,
Thanks for the review. Comments inlined below.
Alan Bateman wrote:
Minor nit but I assume you can initialize perf as:
private static final Perf perf =
AccessController.doPrivileged(new Perf.GetPerfAction());
Fixed.
David Holmes wrote:
Just to add 2c to Alan's method naming comments:
Alan Bateman said the following on 09/13/09 18:07:
Method naming is hard (and often subjective) but there are updates
like this:
PerfCounter.getParentDelegationTime.inc(t1 - t0);
which might be easier to read as:
PerfCounter.getParentDelegationCounter().addTime(t1 - t0)
There's always a problem with method naming when a thing can be both a
counter and a time-tracker. I would agree to use context specific
methods even at the expense of some redundancy in the public API eg:
addTime(long interval)
addElapsedTimeFrom(long startTime)
Also:
PerfCounter.getZipFileCount().inc();
which might be cleaer as:
PerfCounter.getZipFileCounter().increment();
I'd also suggest (in a convention used elsewhere eg
java.util.concurrent.atomic.*) that you reserve increment() for
addition of 1 and use add(long val) for the general case.
I agree that addTime(long interval), increment(), and add(long val) are
better method names. I'll make the change.
As for PerfCounter.getParentDelegationTime() vs
getParentDelegationCounter(), I would prefer to keep
getParentDelegationTime as the method name so that it's clear that this
counter is a time tracker. Similarly, it's clear that getZipFileCount()
is a counter to keep track of zip file count.
Thanks
Mandy
