[google-guice] fizbin commented on revision r663.
Details are at http://code.google.com/p/google-guice/source/detail?r=663

Score: Positive

General Comment:
I'm a bit surprised that resolving only when necessary is such a big win,  
but that's what profiling is for.

I don't know how I feel about combining TypeLiteral and TypeResolver - I  
liked the distinction that TypeLiteral was just a value object token  
without much logic to it.

On the other hand, this change doesn't actually add any state to  
TypeLiteral, so it still isn't that much more than a value object now, so I  
guess it's fine to collapse the two classes.

Line-by-line comments:

File: /trunk/src/com/google/inject/TypeLiteral.java (r663)
===============================================================================

Line 99:   static TypeLiteral<?> fromSuperclassTypeParameter(Class<?>  
subclass) {
-------------------------------------------------------------------------------
While you're here anyway, do you think you could convert the one place that  
uses this method (the protected constructor for Key) over to using other  
more generic (and public) methods?

Respond to these comments at  
http://code.google.com/p/google-guice/source/detail?r=663
--
You received this message because you starred this review, or because
your project has directed all notifications to a mailing list that you
subscribe to.
You may adjust your review notification preferences at:
http://code.google.com/hosting/settings

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"google-guice-dev" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/google-guice-dev?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to