[google-guice] jmourits commented on revision r714.
Details are at http://code.google.com/p/google-guice/source/detail?r=714
Score: Positive
Line-by-line comments:
File:
/trunk/extensions/assistedinject/src/com/google/inject/assistedinject/Assisted.java
(r714)
===============================================================================
Line 35: String value() default "";
-------------------------------------------------------------------------------
javadoc?
File:
/trunk/extensions/assistedinject/src/com/google/inject/assistedinject/Factories.java
(r714)
===============================================================================
Line 57: private static final Class[] ONLY_RIH = {
RealInvocationHandler.class };
-------------------------------------------------------------------------------
put this at the end of the class, since it's not very interesting?
Line 104: * is not bound in the injector.
-------------------------------------------------------------------------------
in the "main" injector?
Line 126: * factory cannot be used until it has been initialized.
-------------------------------------------------------------------------------
until it has been initialized? do you mean until after the injector has
been initialized?
Line 145: * parameters. The names must be applied to the factory
method's parameters:
-------------------------------------------------------------------------------
i like this - nicer than the ordering matching we were doing before
Line 172: * @param factoryInterface a Java interface that defines one or
more create methods.
-------------------------------------------------------------------------------
i don't remember why we did this before - why do we support the "or
more" case? Does it actually come up much?
File:
/trunk/extensions/assistedinject/test/com/google/inject/assistedinject/FactoriesTest.java
(r714)
===============================================================================
Line 103: return "vroom!";
-------------------------------------------------------------------------------
noise?
Line 127: public void testFactoryWithMultipleMethods() {
-------------------------------------------------------------------------------
testFactoryUsesConstructorAnnotatedWithInject?
testFactoryUsesInjectedConstructor?
Line 162: public void
testConstructorDoesntNeedAllFactoryMethodArguments() {
-------------------------------------------------------------------------------
I'm not sure if I like this feature...why not mandate that the argument
lists have to match?
Line 389: public static class MultipleConstructorDefectiveCar implements
Car {
-------------------------------------------------------------------------------
unused?
Line 406: public void testWildcardGenerics() {
-------------------------------------------------------------------------------
neat
Line 475: assertEqualsBothWays(carFactory, carFactory);
-------------------------------------------------------------------------------
do you need to test hashCode also?
Line 483: public void testInjectingProviderOfParameter() {
-------------------------------------------------------------------------------
It's a bit weird to me to inject Assisted params into fields... not sure
how I feel about this.
Line 553: public void testInjectDeepIntoConstructedObjects() {
-------------------------------------------------------------------------------
this is kinda neat
Line 584: public void testDistinctKeys() {
-------------------------------------------------------------------------------
add a negative test?
Respond to these comments at
http://code.google.com/p/google-guice/source/detail?r=714
--
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
-~----------~----~----~----~------~----~------~--~---
