Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is there a reason why TyphoonDefinition Types can't be modified at runtime? #502

Open
pkrmf opened this issue May 11, 2016 · 13 comments
Open
Labels

Comments

@pkrmf
Copy link

pkrmf commented May 11, 2016

Let's imagine I have an assembly with a bunch of TyphoonDefinitions that assemble my objects. All of those definitions implement different protocols and provide those objects through default implementation classes.

Now I want to swap those implementation classes at runtime, which means that the Type of a TyphoonDefintion is going to change to the new type as long as that type implements the same protocol that the old type was implementing.

I can always ask my TyphoonComponentFactory that initialized with my Assemblies to give me the registry, and through that get all the Implementation classes for every Definition.

Is there any reason why I could not swap that Definition Type with a new Type that conforms the same protocol? (Same init methods, same protocols, same everything...).

I made the code change in my own workspace and its behaving as expected but I have concerns that I could possibly be breaking something somewhere else.

@alexgarbarev
Copy link
Contributor

I think you can use Typhoon Option Matcher feature for that reason: https://github.com/appsquickly/Typhoon/wiki/Typhoon-Option-Matcher - You can define two definitions with different Types and then another - OptionMatcher definition which will point to one of these based on runtime argument, or runtime value.

If definitions has same injections, you can inherit that using "parent" property but specify different Type

Alternatively you can use own factory to create instance using "withFactory: configuration" method that gives you maximum flexibility and most verbose method.

So there is plenty solutions to achieve that, it's up to you which way to go.

@pkrmf
Copy link
Author

pkrmf commented May 12, 2016

Thanks @alexgarbarev option 2 is what I am exactly looking for but I am still having some issues making this to work.

I create a definition based on the parent, which has a Class defined, a Key defined and a scope defined.

Do you have any idea why when I create my new definition based on parent and with the new class, it doesn't register the same key that the parent definition had? Instead my key is null, and when I try to use the patcher to replace definitions it crashes because at some point is trying to set a nil object into a dictionary.

`(lldb) po defintion
TyphoonDefinition: class='Device', key='device', scope='ObjectGraph'

(lldb) po newDefinition
TyphoonDefinition: class='FakeDevice', key='(null)', scope='ObjectGraph'

[UsaaFactoryTests testRegisteredComponentClass] : failed: caught "NSInvalidArgumentException", "*** setObjectForKey: key cannot be nil"`

@alexgarbarev
Copy link
Contributor

Can you show me your assembly code?

@pkrmf
Copy link
Author

pkrmf commented May 12, 2016

-(__autoreleasing id<IDevice>)device { return [TyphoonDefinition withClass:[Device class] configuration:^(TyphoonDefinition *definition) { [definition useInitializer:@selector(initWithName:) parameters:^(TyphoonMethod *initializer) { [initializer injectParameterWith:@"REAL NAME"]; }]; }]; }

Then I register this assembly with the TyphoonComponentFactory so I can take advantge and ask for assembled objects that implement IDevice Protocol.

Once my Factory has been initialized with all the other assemblies:

TyphoonDefinition *newDefinition = [TyphoonDefinition withParent:definitionCopy class:newRegisteredClass];
                        TyphoonPatcher *patcher = [[TyphoonPatcher alloc] init];
                        [patcher postProcessDefinition:newDefinition replacement:&definitionCopy withFactory:_componentFactory];

Where definitionCopy is the default definition for device.

@alexgarbarev
Copy link
Contributor

You must define you definition with parent as another method inside TyphoonAssembly. This is the only way to get 'key' generated correctly.

If you want to have two definitions with different Types, and you want to get appropriate definition based on runtime value, try this approach:

- (id<IDevice>)device
{
    return [TyphoonDefinition withClass:[Device class] configuration:^(TyphoonDefinition *definition) {
        definition.autoInjectionVisibility = TyphoonAutoInjectVisibilityNone;
    }];
}

- (id<IDevice>)alternativeDevice
{
    return [TyphoonDefinition withParent:[self device] class:[AnotherDevice class] configuration:^(TyphoonDefinition *definition) {
        definition.autoInjectionVisibility = TyphoonAutoInjectVisibilityNone;
    }];
}

- (id<IDevice>)currentDevice
{
    return [TyphoonDefinition withOption:runtimeBooleanValue matcher:^(TyphoonOptionMatcher *matcher) {
        [matcher caseEqual:@YES use:[self device]];
        [matcher caseEqual:@NO use:[self alternativeDevice]];
    } autoInjectionConfig:^(id<TyphoonAutoInjectionConfig> config) {
        [config setAutoInjectionVisibility:TyphoonAutoInjectVisibilityByProtocol];
        [config setClassOrProtocolForAutoInjection:@protocol(Device)];
    }];
}

of if you want to resolve using assembly interface, try this:

- (id<IDevice>)currentDeviceAlternative:(NSNumber *)alternativeBoolNumber
{
    return [TyphoonDefinition withOption:alternativeBoolNumber matcher:^(TyphoonOptionMatcher *matcher) {
        [matcher caseEqual:@YES use:[self device]];
        [matcher caseEqual:@NO use:[self alternativeDevice]];
    } autoInjectionConfig:^(id<TyphoonAutoInjectionConfig> config) {
        [config setAutoInjectionVisibility:TyphoonAutoInjectVisibilityByProtocol];
        [config setClassOrProtocolForAutoInjection:@protocol(Device)];
    }];
}

@pkrmf
Copy link
Author

pkrmf commented May 12, 2016

@alexgarbarev But on these situations that you are exposing I need to know about the implementations that can be swappable at compile time since I need to know that there is a alternativeDevice class that live somewhere.
Let's think about a scenario where a consumer of my Network framework wants to swap one of the implementation classes(HTTPClient for example) that I provide with his own AnyClass. At compile time on my Network Framework I cannot know about his AnyClass, since I don't know about AnyClass since it will probably exist on the consuming application.
As long as they are implementing the same interface that my objects are implementing in my assemblies, I would allow them to swap the default class with his own class.

@pkrmf
Copy link
Author

pkrmf commented May 12, 2016

I see different options here but I don't know what other things could break.

I can always create a TyphoonDefinition at runtime with the parent and the new class, and then If i have the TyphoonDefinition+Infrastructure category public, i can set the key to be the same as the parent so at that point I could use the patcher to replace the definitions.

The other option is to make the Type of a definition readwrite, and modify the existing definition directly to support another type. I don't know how dangerous this second option could be...

@alexgarbarev
Copy link
Contributor

I think legal way to achieve that dynamism is custom factory via [TyphoonDefinition withFactory:]. But it's up to you how to do that.

@pkrmf
Copy link
Author

pkrmf commented May 12, 2016

I am not quite understanding the TyphoonDefinition withFactory option. Does it mean I need to create my TyphoonDefention withFactory at runtime with the new class? Or that should be defined in my assembly?

@pkrmf
Copy link
Author

pkrmf commented May 12, 2016

@alexgarbarev Do you happen to have an example on TyphoonDefinition WithFactory where it would work?

@pkrmf
Copy link
Author

pkrmf commented May 12, 2016

I just wanted to provide a way to the consumer of all my "Frameworks" to be able to replace TyphoonDefinitions at runtime without having to know what a typhoon definition or even Typhoon is. I am trying to make the life of the consumer the easiest as possible and I don't want to require a consumer to know how to create assemblies and make definitions.

I have a Factory class that has a @Property TyphoonComponentFactory. I register all the available Assemblies(As many as Frameworks the consumer imported) to the TyphoonComponentFactory so I can take advantage of retrieving components by protocol.(Let's think a complex system where some of these Frameworks have dependencies on each other).

At this point I have a nice system where the consumer says:
id<IHTTPClient> httpClient = [Factory getComponentForProtocol:@protocol(IHTTPClient)]

and he gets the default implementation of IHTTPClient(HTTPClient Class) that was assembled through Framework A assembly.

But we can do even cooler things, the consumer can ask the Factory to register a single component, without the need of knowing about typhoon:
[Factory registerComponent:[FakeHTTPClient class] forProtocol:@protocol(IHTTPClient)]

At runtime, as long as he is implementing IHTTPClient in his new FakeHTTPClient class I can ask the TyphoonComponentFactory if he has a TyphoonDefinition with a Type that conforms to IHTTPClient. If it does, I could create a copy of that definition, set the Type to the FakeHTTPClient class and attach it as a post processor, but the problem is that Type is a readonly @Property in TyphoonDefinition.

I am still trying to get my head around and find a better solution other than changing the Type of a TyphoonDefinition to be readwrite but I don't see how your solutions could help me on this scenario(maybe I just don't know how to implement them).

Would a pull request for making Type readwrite be too crazy?

@alexgarbarev
Copy link
Contributor

Actually I can't see scenario when you need to create definitions at runtime. I don't think that you generate these classes at runtime, so I prefer definitions made inside assemblies, before compile.

This API:

[Factory getComponentForProtocol:@protocol(IHTTPClient)]
[Factory registerComponent:[FakeHTTPClient class] forProtocol:@protocol(IHTTPClient)]

looks like other DI containers had, and thought it's possible to achieve with Typhoon, I think assemblies interface is much better:

  1. Better to resolve using method name, instead of type (so you can have more than one definition for same type)
  2. Much nicer/understandable code for review. Anyone can see all dependencies and definitions by reading Assembly file. No need to search and review complex "register:forProtocol" calls.
  3. Easier to see usages of concrete definition (scan for method caller inside IDE)

If you still want to preceed and replace definitions at runtime, you can go with

- (void)enumerateDefinitions:(void(^)(TyphoonDefinition *definition, NSUInteger index, TyphoonDefinition **definitionToReplace, BOOL *stop))block;

method of TyphoonComponentFactory.

@pkrmf
Copy link
Author

pkrmf commented May 13, 2016

@alexgarbarev I totally understand your point. But we might have different business cases on why I would like to provide or not the source of an Assembly to the consumer of my Framework(which I have no problem doing obviously). But for me having to tell the consumer that every time he wants to swap the implementations that I provide he needs to create his own assemblies seems a lot of work.

Imagine this simple case where I have a Device Framework that returns the modelName as a NSString.
Here is the assembly:

-(id<IDevice>)device {
       return [TyphoonAssembly withClass:[Device Class]];
}
-(NSString *)modelName {
     return [TyphoonAssembly withFactory:self.device sel:@selector(modelName)];
}

And then the Device class would implement the logic on what NSString return for a modelName.

Imagine then a Network Framework has a dependency on that Device Framework because in every Network call theres the need to attach the modelName as a header. I can provide assemblies for both Netowrk Framework and Device Framework which will implement the same interface that the Frameworks provide. The consumer doesn't even know those assemblies exist, he can see What a Device is and what protocol implements.

Now one of my consumers decides that he doesn't like how I name the devices(yeah its dumb) and he wants to implement his own logic to return a different modelName, maybe he just want to add prefixes to it who knows. He has to create a TyphoonAssembly subclass, create a definition for (id)Device and another for modelName. Along with all this, he needs to create his FakeDevice Class that implements the modelName to return something different that I was providing. Then he has to retrieve the TyphoonDefinition of his own Assembly(Another API) and pass it to my Factory class that is smart enough to replace definitions at runtime.

Now the Factory can swap definitions, and my Network Framework will resolve dependencies with his FakeDevice implementation instead of the default i was providing which is great. But he had to know about typhoon, TyphoonAssemblies, TyphoonDefinitons and a bunch of other different APIs.

In a perfect world(or maybe just my perfect world), he would just want to Define a new FakeDevice class that implements the same protocol as my Device. He would just give his own implementations and tell the Factory he wants to use this FakeDevice instead of the default one that I provide.

Here is where I am trying to go, maybe Typhoon should be flexible enough to provide this type of functionality, because there might be different business cases. I proofed that this is achievable, but it gives me the concern of what could go wrong making the Type of a TyphoonDefinition readwrite.

1) Better to resolve using method name, instead of type (so you can have more than one definition for same type) I have easy ways to provide this functionality with FactoryObjects(I can go deep into that if needed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants