On 4.8.14 10:47 , Davide Giannella wrote:
On 04/08/2014 09:35, Michael Dürig wrote:
One thing that concerns me is the static reference to the executor
service. It might keep the JVM from terminating properly as it is
never shut down. A better approach might be to create it on the fly in
the start method and shut it down in the stop method. If this is too
much overhead we should make it an instance field and have the class
implement Closeable.
I see what you mean and it was my concern as well. Anyhow I decided to
go this way and see if anyone would have my same concern.
I'm more for the class instance and the Closable interface as if you
rely on stop() some clients might not use it (you could use only
split()) and therefore I would implement closable anyhow.
Agreed, I think this is the better approach.
As a further improvement it might be helpful if clients could inject
the logger instance instead of always using the StopwatchLogger one.
The idea was to have a single appender where you says: enable all the
timing and then filter (grep) the log for your own cases. But adding a
constructor with an instance variable for logging should be trivial.
Sure. This was just an idea. We can as well leave it out and add it once
we think such a thing is needed.
Michael
Anyhow here is the amended version
http://goo.gl/xUcPdY
D.
