http://gwt-code-reviews.appspot.com/719801/diff/5001/6011 File dev/core/src/com/google/gwt/dev/util/log/speedtracer/SpeedTracerLogger.java (right):
http://gwt-code-reviews.appspot.com/719801/diff/5001/6011#newcode72 dev/core/src/com/google/gwt/dev/util/log/speedtracer/SpeedTracerLogger.java:72: if (data == null) { On 2010/07/27 22:39:38, scottb wrote:
Actually, how could data ever be null? These are always invoked
through varargs
invocations. Same for line 85.
new Event(null, null, null) trips the null case in the if test. I could pass a pair of empty strings here, but I don't see a good argument for making the API more brittle at the expense of an extra null check. I changed this constructor from String[] to String... and it didn't help. I wish the null case was handled in the Lists class - it leaves the Lists class light, but now all the cod e that calls it has to conditionally call create() or create(T...). line 85 is a different matter - that is just unnecessary so I removed it. http://gwt-code-reviews.appspot.com/719801/diff/5001/6011#newcode133 dev/core/src/com/google/gwt/dev/util/log/speedtracer/SpeedTracerLogger.java:133: return singleton; On 2010/07/27 22:39:38, scottb wrote:
I'm actually totally confused by this discussion. With this set of
changes,
it's no longer ever possible to change the value of singleton at all,
right? So
how about just make it a static final auto-init in SpeedTracerLogger
and make
init() do nothing? Then you don't even need the get() method anymore,
just ref
the static final field.
I think this pattern is interesting. Implemented in this way, init() does what it says. If there were some other call to SpeedTracerLogger (say, calling the Constructor directly as in the Unit test) it would auto-initialize the static. As an aside, I'm thinking this is not the last time we will visit this code. I'm thinking that we are going to have to do something different than what we do now on initialization for distributed compilation. We may have to write the speedtracer logs to a unique filename somehow (maybe encode the permutation name and step name). http://gwt-code-reviews.appspot.com/719801/diff/5001/6011#newcode216 dev/core/src/com/google/gwt/dev/util/log/speedtracer/SpeedTracerLogger.java:216: * {...@link #get()} is not affected. On 2010/07/27 22:39:38, scottb wrote:
get() is now private, so this doc should probably get deleted.
This is not the current version of the patch. newInstance() is gone altogether. http://gwt-code-reviews.appspot.com/719801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
