Definition inheritance

[rhgills]

Spring-style. See http://docs.spring.io/spring/docs/3.2.4.RELEASE/spring-framework-reference/html/beans.html#beans-child-bean-definitions.

Basically: Specify a parent definition.

Block syntax would be something like:

- (id)knight
{
    return [TyphoonDefinition withClass:[Knight class] properties:^(TyphoonDefinition* definition)
    {
        [definition injectProperty:@selector(quest) withDefinition:[self defaultQuest]];
        [definition injectProperty:@selector(damselsRescued) withValueAsText:@"12"];
        [definition setScope:TyphoonScopeDefault];
    }];
}

- (id)kingArthur
{
    return [TyphoonDefinition withProperties:^(TyphoonDefinition* definition)
    {
        definition.parent = [self knight];

        [definition injectProperty:@selector(damselsRescued) withValueAsText:@"502"];
    }];
}

For XML it can be a simple parent="definitionName" attribute inside the component element.

[jasperblues]

100% agree with this feature. . . it gives a very good ROI on development effort.

[rhgills]

Implemented initializer inheritance only (no property inheritance) in 65ba872.

If a child specifies any initializer block (including an empty one) as opposed to nil, the child's initializer overrides the parents. No inheritance of the parent's class yet, either.

[jasperblues]

Nice work so far :)

[rhgills]

Thanks!

One thing I don't like is the obscurity of the unit tests. Consider the following test:

- (void)test_child_missing_initializer_inherits_parent_initializer_with_parameters
{
    DefinitionInheritanceAssembly* factory = [self newDefinitionInheritancefactory];

    Knight *childKnight = [factory childKnightWithConstructorDependency];

    assertThat(childKnight, instanceOf([Knight class]));
    assertThat(childKnight.quest, instanceOf([CampaignQuest class]));
}

The meaning of this test is unclear without looking at the DefinitionInheritanceAssembly code. Just from looking at the test, we don't know if what we're verifying is actually correct. The test itself doesn't tell us if childKnightWithConstructorDependency actually creates a knight. The test also does not tell us that the childKnightWithConstructorDependency definition specifies that a CampaignQuest be injected into the initializer during construction of the Knight. I tried my best to make this clear with names, but I feel it could be improved by constructing a minimal assembly in the tests.

If you're familiar with XUnit Test Patterns, the DefinitionInheritanceAssembly is an immutable shared fixture. We should be constructing a minimal fixture in each test.

So the test used as an example above could be changed to:

- (void)test_child_missing_initializer_inherits_parent_initializer_with_parameters
{
    DefinitionInheritanceAssembly* factory = [self newDefinitionInheritancefactory];

    //
    // register campaign quest definition here
    //

    // could add directly on the factory by having the factory later construct the definition itself from class and initializer block
    [factory addDefinitionForKey:@"parent" class:[Knight class] initializer:^(TyphoonInitializer *initializer) {
            initializer.selector = @selector(initWithQuest:);

            [initializer injectWithDefinition:[factory definitionForKey:@"campaignQuest"]];
        }];

    // or perhaps register a block with the factory as a more direct replacement for a method returning a definition
    TyphoonBlockDefinition childBlockDefinition = ^(TyphoonComponentFactory *factory){
        return [TyphoonDefinition withClass:[Knight class] properties:^(        
        TyphoonDefinition *d) {
            d.parent = [factory definitionForKey:@"parent"];
        }];
    };
    [factory addDefinitionWithBlock:childBlockDefinition forKey:@"child"];

    Knight *child = [factory objectForKey:@"child"];

    assertThat(child, instanceOf([Knight class]));
    assertThat(child.quest, instanceOf([CampaignQuest class]));
}

And from this very verbose base, we could extract some test utility methods to abstract away some details.

- (void)test_child_missing_initializer_inherits_parent_initializer_with_parameters
{
    DefinitionInheritanceAssembly* factory = [self newDefinitionInheritancefactory];
    [factory registerCampaignQuestDefinition]; // not directly relevant to test, so OK to extract
    id parentKey = [factory registerKnightHavingCampaignQuestInjectedIntoConstructor];
    id childKey = [factory registerChildWithParentHavingKey:parentKey];

    Knight *childKnight = [factory objectForKey:childKey];

    assertThat(childKnight, instanceOf([Knight class]));
    [self assertThatQuestConstructedFromCampaignQuestDefinition:childKnight.quest];
}

You still need to drill down to test utility methods to fully understand the test, but it should be clear from their names what they do - and, unlike before, the test is explicitly involved in setting up the assembly. This solves the problem of having a large assembly with many definitions of which only some of which are relevant to the current test. The use of test utility methods to dynamically create assemblies in this way gives us lots of flexibility.

I'd love to hear any thoughts on the idea in general and on which method of adding a definition to the assembly you prefer. Namely:

• having the assembly construct a definition for us, lazily, from the same parameters that you usually pass to the definition constructor
• passing a block to the assembly which returns a definition

[jasperblues]

Hey Robert,

Yes, I agree with this.

I think the problem is that there are some (aka a lot) integration tests that should really be unit tests. . . ie we’re building XML or ‘block'-style assemblies, in order to test lower-level functionality. . . I think the integration tests still definitely have value, but a lot of them could be moved down as unit tests.

I’m not sure that I understood what you meant by this:

having the assembly construct a definition for us, lazily, from the same parameters that you usually pass to the definition constructor
passing a block to the assembly which returns a definition

If you’re asking about the different ways to construct a definition the idea was this:

The alloc/init style is verbose and for internal use.
The class/factory methods (with blocks) are more terse, and also provide a way to group related information.

Is this what you meant? Were you suggesting this could be changed / improved ?

[rhgills]

I just came to the same realization myself that we're testing behavior of the component factory by building assemblies. I agree that we should reduce the amount of integration tests, covering perhaps only a few relevant cases end-to-end, while moving the rest of the tested functionality down to the unit test level.

In regards to your question, I was trying to figure out how to register a definition with the component factory and completely overlooking the simplest and best solution: the register: method. I was thinking that I couldn't just statically construct a definition with a dependency on another definition, but of course you can. You just have to do so in the correct order. Perhaps I've been using the block-style assemblies too much. ;)

Since we last spoke, I've:

• added basic definition inheritance to the XML style assembly
• replaced the integration tests covering definition inheritance with unit tests on the ComponentFactory (in ce4e4e8). Alongside the AsComponentDefinitionTests, these cover both the block and XML based approaches. I still think there is value to having a single, simple parent inheritance integration test (child inherits parent initializer) for both the block and XML assemblies, which I'm planning on adding back in later.
• refactored the AsComponentDefinitionTests for inheritance to construct an XML element in the test themselves, instead of using a .xml assembly file

I'm very curious to hear any thoughts on replacing these integration tests with unit tests and with the XMLBuilder used in AsComponentDefinitionTests.

[jasperblues]

This is done, right?

[rhgills]

In a very basic state. There are four tests in TyphoonComponentFactoryTests which cover the behavior implemented right now. The initializer will be inherited from a definitions parent, if none is specified for the child. But, no inheritance occurs for properties (yet). And specifying an initializer on the child will completely override the parents initializer.

Improving both of these is on my TODO list.

[jasperblues]

"And specifying an initializer on the child will completely override the parents initializer."

What would be the most self-evident behavior here? Perhaps we should follow the Spring behavior? (this is a Spring-ish feature)

"But, no inheritance occurs for properties (yet)."

Let's leave it open. I think we can call it finished once we can inherit, and optionally override specific properties as well.

[jasperblues]

Hi @rhgills, is this ready to use? because @mivasi is keen to use it ..

Shall we get this documented?

[rhgills]

No progress on this since we last spoke. Now that #107 is winding down, this is next up. Sorry for the glacial pace. I'll get this finished up ASAP.

[jasperblues]

Hey no worries man! . . . was just checking. . . we're all volunteers here.

@mivasi is checking out what's there so far. . . he also needs the new ObjectGraph scope - i'm planning to implement that ASAP.

[rhgills]

Yup, sorry: that sounded a bit terse now that I reread it! My bad! But I do want to get this done ASAP; it'll let me deduplicate a substantial amount of code in one of my own projects.

Always good to know that we have some real users wanting the features that are top priority. 👍

[jasperblues]

Haha - it didn't sound terse. . I just didn't want you to feel under pressure.

@mivasi actually wrote what I believe was the first open-source DI container for Objective-C. . . it was the first one I came across anyway - back in 2010. . . so its great that he's helping us test out the new features.

[rhgills]

No worries, no pressure here! :)

And that is great! Glad to have you here, @mivasi.

[jasperblues]

Just tried this and its not working for me. . . . the way I would implement this feature would be to copy all of the attributes from the parent definition(s) to the child during start-up. . . rather than trying to introspect this at runtime.

[rhgills]

What's not working?

And thanks, that seems like a better approach, I'll explore that!

[jasperblues]

I didn't get time to create a failing test - sorry. . But here's what I tried:

• Used latest master in an App.
• Created a ‘abstract’ definition (aware that nothing currently prevents its instantiation). Created two components that extend from it. Didn’t work. . . deleted the parent def, created two with duplicate properties. worked.

[jasperblues]

Implemented at definition level, instead of in instance builder.