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

Guidance on objects being instantiated before application:didFinishLaunchingWithOptions:... where, when, or how should services be set up? #308

Closed
fatuhoku opened this issue Jan 17, 2015 · 21 comments

Comments

@fatuhoku
Copy link
Contributor

Many services offer iOS SDKs that require the app to set the SDK up with a one-line class method call early in the application. e.g.: Parse's initialization looks like: [Parse setApplicationId:@"<your app id>" clientKey:@"<your client key>"];.

These often go into the -application:didFinishLaunchingWithOptions: method of your XXAppDelegate.

When using Typhoon, it sometimes makes sense to inject components that make use of these services. The problem is, it's possible that these components are instantiated before -application:didFinishLaunchingWithOptions: even has a chance to execute. For instance:

@interface MYAppDelegate ()
@property(nonatomic, strong) InjectedClass(MYComponentThatNeedsParseSetUpBeforeInitialization)component;  // !!! calls to Parse made before Parse is ready
@end

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions {

    [Parse setApplicationId:@"<your app id>" clientKey:@"<your client key>"];

    // Parse is ready here.

    return YES;
}

So officially, where should these calls go instead?

@jasperblues
Copy link
Member

We could debate creating a hook for this in either the AppDelegate:

- (void)typhoonWillStartUp; //for want of a better name

or TyphoonAssembly:

- (void)assemblyWillAssemble; //or something

*but what I currently do is create a category on the framework class and use: *

[definition performBeforeInjections:SEL];

If you wish you can use performBeforeInjections:parameters and have the parameters come from TyphoonConfig

@alexgarbarev
Copy link
Contributor

another options:

  1. Create some "wrapper" for Parse static calls (for example ParseManager or something), then create definition for that wrapper with singletone scope - after you can inject that wrapper into your component that need Parse. (i.e. explicit dependency)

<-- Useful when not too much objects requiers parse at startup

  1. Create definition for class that conforms to TyphoonDefinitionPostProcessor. This definition would be registered as factory "post-processor". All "post-processors" runs at very start, before singletons creation.
    Then you can do all your configurations that need to performs at very start inside postProcessDefinitionsInFactory: method

<-- Harder to read and understand (implicit) but usefull when too much objects requires one dependency (for example logging system like cocoalumberjack)

@jasperblues
Copy link
Member

And one other existing way folks do this is create a definition that uses @selector(class) as its initializer. Eg:

- (id)loggers
{
    return [TyphoonDefinition withClass:[DDLog class] configuration:^(TyphoonDefinition *definition) {
        [definition useInitializer:@selector(class)];
        [definition injectMethod:@selector(addLogger:) parameters:^(TyphoonMethod *method) {
            [method injectParameterWith:[self ttyLogger]];
        }];
        definition.scope = TyphoonScopeSingleton;
    }];
}

This definition above has singleton scope, so will be run on startup. It won't be guaranteed to run first though, so take care. (Subsequent components that depend on this could have TyphoonScopeLazySingleton, TyphoonScopeWeakSingleton, TyphoonScopeObjectGraph (default) or TyphoonScopePrototype).

Questions:

  • Given all of these approaches do we need a standard approach? Or is it OK to be creative? Perhaps extract a wiki page and link it from the 'configuration' section of What can be Injected ?
  • Will the hooks suggested above add more value than the complexity they introduce?

@fatuhoku
Copy link
Contributor Author

@jasperblues Thanks for the response. I'll try the latter approach out. It seems to be more consistent with other definitions. [definition useInitializer:@selector(class)]; feels like a hack though so there could be better syntax for this.

About a standard approach, I think yes, definitely. And yes, it is okay to be creative as well, only if the standard approach isn't sufficient for your needs. This sort of thing is in virtually every app. All the developer cares about is that I just want, say, Parse to be set up in my app so that I can start using it. The last thing I want is to be forced to consciously think about how to teach my fantastic Typhoon DI container to do something that normally takes 2 seconds: paste a line of code into -application:didFinishLaunchingWithOptions.

So yes! A documented standard approach please!

@jasperblues
Copy link
Member

Which to people like:

- (void)typhoonWillStartUp; //for want of a better name
``

or TyphoonAssembly:

```objc
- (void)assemblyWillAssemble; //or something

or The way shown above using the 'class' as the initializer along with the ability to set order or instantiation?

My preference is the 2nd: asssemblyWillAssemble

@fatuhoku
Copy link
Contributor Author

Yeah I imagine the latter is better. Parse is a dependency; and therefore falls under the remit of — guess what — a dependency injection framework like Typhoon!

So I actually see that Typhoon will recommend not one, but two potential implementation options:

  1. the super-quick cut-and-paste solution: -assemblyWillAssemble execute prior to instantiating any components in that assembly. Paste code from -application:didFinishLaunchingWithOptions: and we are done. Fantastic. The downside to this implementation is of course that API keys and other bits of configuration can't get injected.
  2. the "But I want DI to inject my configuration, dude" solution: if the developer wants to switch between API keys then they can go and use the [definition useInitializer:@selector(class)]; approach you highlighted just now.

I would stress that both options need to be clearly documented so that the developer has a chance of picking and choosing what's suitable for them at the time. The second option is the 'correct' way to set it up, but requires knowledge of Typhoon and is premature abstraction.

It's also worth saying at this point that this issue only exists if you want to inject anything into AppDelegate or something else sufficiently early. If your components get initialised and injected only on your first UIViewController being loaded then this is not an issue.

What do you think @jasperblues?

@alexgarbarev
Copy link
Contributor

@fatuhoku
The problem with [definition useInitializer:@selector(class)]; solution is that you cannot control time when it's performed. I had that problem using this approach, some of my singletones tries to use logging system which was not initialized yet.

Maybe we can extend API or something... After that we could recomment that as 'correct' approach

@jasperblues
Copy link
Member

  • Having eg [definition setIntantiationPriority:1]; //default is 100 . . allows fine grained control but is messy since it would only apply to singletons. Would have to raise error in an attempt to use this for non-singleton. In retrospect it might've been nice if we had sub-class per scope. eg TyphoonObject (default), TyphoonPrototype , TyphoonSingleton . . this would allow specific properties for that type.
  • Introducing TyphoonScopeEagerSingleton could work.

@alexgarbarev
Copy link
Contributor

Finally I think using assemblyWillAssemble is more obvous is easy to use solution because using priorities is too implicit - hard to read and understand the order.. Also that makes dependencies implicit and hard to understand too.

I prefer these two options:

  1. Make wrapper (as I told in my first answer in this thread) - that makes dependencies explicit (correct way)
  2. Make assemblyWillAssemble or anything similar - some peice of code that guaranteed to be peformed at start. That makes dependencies implicit but easy to understand, since all in once place (simple way)

@jasperblues
Copy link
Member

@fatuhoku Would you like push access to help clean up the documentation? It would be really helpful, since the challenges you faced getting started with Typhoon (current features) are still fresh in mind. Meanwhile, you've been involved in discussions on future features.

@fatuhoku
Copy link
Contributor Author

@jasperblues Yeah I'm not objected to that. I would love to contribute what I know of Typhoon in the form of documentation. I intend to slowly refactor my application to make use of it.

@jasperblues
Copy link
Member

@fatuhoku granted. We really appreciate your help. Its nice to meet someone who cares about good documentation. As it stands now Typhoon is helpful for folks are very familiar with Di from other languages. We'd really like if the documentation was good enough for all folks to benefit.

@jasperblues
Copy link
Member

Please let us know if you make changes so that we can review the technical accuracy and give feedback.

@fatuhoku
Copy link
Contributor Author

@jasperblues Sure thing. I'll do this on a need-by-need basis. Got a slightly different focus right now!

@jasperblues
Copy link
Member

@fatuhoku I just faced this issue too. Cheated by hooking resolveCollaboratingAssemblies

@fatuhoku
Copy link
Contributor Author

@jasperblues Ah, look at you, hooking into all those undocumented methods ;) Being creator has its perks! If you're happy with that as the solution for now I could go document it.

@alexgarbarev
Copy link
Contributor

I found that init method on AppDelegate works perfectly on our needs, like this:

@implementation AppDelegate

- (instancetype)init
{
    self = [super init];
    if (self) {
        [self setupLogging];
    }
    return self;
}

The lifecycle is:

  1. main.m
  2. UIApplication creation
  3. AppDelegate creation <-- init method called here
  4. UIApplication setDelegate called <-- Typhoon created here
  5. UI Loading
  6. Application did finish launching called

@jasperblues
Copy link
Member

I wondered if it still might be beneficial to create the life-cycle method? This would communicate intention to folks. Alternatively we can just add API doc to init, saying it can be safely overridden.

@fatuhoku
Copy link
Contributor Author

@alexgarbarev Fantastic. I think that has to be the most straightforward solution of all... It is pretty non-standard though.

@jasperblues My vote is for documenting the use of AppDelegate init. Reason being, it's quick, and it's stuff that Typhoon really doesn't need to know about until it needs to.

@jasperblues
Copy link
Member

Once the docs have been done, let's badge as 'Ready for Review'. After that we can close off the ticket.

@jasperblues
Copy link
Member

NB, if you add API docs to any Typhoon classes, and commit, they'll turn up on www.typhoonframework.org/docs/latest/api after a few minutes.

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

No branches or pull requests

3 participants