Hi Scott, I implemented changes w/r/t your feedback:
http://gwt-code-reviews.appspot.com/719801/show On Mon, Jul 26, 2010 at 11:33 AM, Scott Blum <[email protected]> wrote: > On Fri, Jul 23, 2010 at 7:51 AM, <[email protected]> wrote: >> >> I agree that going back to static methods could save a little bit of >> typing, but nevertheless, it is a common pattern for a logging class >> (j.u.logging, log4j). Also, if it is keystrokes that bother you, you >> could save a lot of typing in the invocation with the following pattern: >> >> SpeedTracerLogging stl = SpeedTracerLogger.get(). >> stl.start(...); >> ... >> stl.end(...); > > That feels kind of cumbersome, since you already have to track the Event > instance in order to end the right instance. > >> >> - Go back to all static invocations and then have the test grab >> instances with new() or newInstance and use package protected methods >> with names that don't collide with start() and stop() like startImpl() >> stopImpl(). This doesn't feel right to me - it seems like the test is >> sneaking around under the covers of the API. It also guarantees a use >> of multple speedtracer loggers will require a major refactor. > > > I'm actually most in favor of this approach, because the whole idea of speed > tracer logging entails the idea of a singleton -- at least per thread. If > we wanted to use multiple speed trace logs in the future, it seems almost > certain to me that at least each thread would consistently use a single > logger. Because of this idea of thread locality implicit in the API, that > means we wouldn't need a major refactor. You could imagine: > stl = SpeedTracerLogger.newLogger(outputFile); > stl.setForThread(Thread.currentThread()); > I might also argue that comparing this to j.u.logging or log4j is not the > right comparison. Since this is for performance logging, a more apt > comparison might be to the JProfiler API, which is a static API. > Scott > -- Eric Z. Ayers Google Web Toolkit, Atlanta, GA USA -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
