Singleton root view controller instantiated from storyboard

[HiveHicks]

I want to make my root view controller a singleton, but seems like it's impossible, and I can see why. If I declare a scope of the definition as singleton, Typhoon creates controller first, but then storyboard instantiates it once again, and Typhoon doesn't store reference to it as it's already holding singleton reference.

I've been thinking about it for a while, and I can't think of a way to inject root view controller to any component. Do you have any ideas?

[mogol]

Hey @HiveHicks , could you create/show example code for this issue?

How do you instantiate root view controller first time? second time?

[HiveHicks]

@mogol, I've created a sample project here: https://bitbucket.org/HiveHicks/typhoonsingletoncontroller

ViewController is defined as TyphoonScopeLazySingleton, however root view controller and view controller which is injected into NavigationManager are different:

2015-09-01 13:49:07.691 TyphoonSingletonController[99950:2732354] Root view controller: <ViewController: 0x7fbc486123f0>
2015-09-01 13:49:07.692 TyphoonSingletonController[99950:2732354] Navigation manager view controller: <ViewController: 0x7fbc48571c10>

[mogol]

@HiveHicks, thanks for sample. We have a bug or a feature in instantiating view controllers using storyboards. TyphoonStoryboards always return new instances of view controllers. WA - try not to use Storyboards with Singleton view controller.

We'll fix soon this.

[etolstoy]

FYI: yesterday we've encountered the same issue in our project.

[etolstoy]

I've investigated the problem - and it's a bit deeper than it seems.

We have a bug:

• when injecting an instance created outside of Typhoon using inject: method, the singletone scope is overwritten,

and a problem:

• ViewControllers, when instantiated via storyboard, are always created outside of Typhoon scope, so we can't use the singletone scope for them.

I see two possible solutions:

• We can look for the appropriate ViewController instance in the TyphoonComponentFactory pools by using something like - (id)cachedInstanceForViewController:(UIViewController *)viewController. In the implementation we'll check if this controller is a singletone, if no - then this method will return nil, the controller would be instantiated using [super instantiateViewControllerWithIdentifier:identifier];.
• We can explicitly say, that view controllers definitions scopes don't affect direct storyboard instantiation.

[alexgarbarev]

@igrekde Hmmm.. I thought we retaining ViewController instances created by storyboard inside Typhoon.

See method, called on each created ViewController:

- (void)inject:(id)instance withDefinition:(TyphoonDefinition *)definition
{
    @synchronized (self) {
        id<TyphoonComponentsPool> pool = [self poolForDefinition:definition];
        [pool setObject:instance forKey:definition.key];
        TyphoonStackElement *element = [TyphoonStackElement elementWithKey:definition.key args:nil];
        [element takeInstance:instance];
        [_stack push:element];
        [self doInjectionEventsOn:instance withDefinition:definition args:nil];
        [_stack pop];
        if ([_stack isEmpty]) {
            [_objectGraphSharedInstances removeAllObjects];
        }
    }
}

Calling this:

[pool setObject:instance forKey:definition.key];

should put instance into correct objects pool.. Is that you looking for?

[etolstoy]

@alexgarbarev I'm not sure that it's a correct behavior:

• We have a definition for MyViewController with the TyphoonSingleton scope. This controller is already injected somewhere.
• When we open this controller using a storyboard, a new instance is created: id viewController = [super instantiateViewControllerWithIdentifier:identifier];
• And then, [pool setObject:instance forKey:definition.key]; overrides our previously created singleton, replacing it with a new object.

That's why I'm thinking of a two-step view controller instantiation:

1. We look for the singleton instance in component pools, and if there is any, we return it.
2. If there are no singleton instances, we return the original one.

[alexgarbarev]

I just checked demo project. The problem is following:

1) App launched, we injecting ViewController class instance into AppDelegate, retaining into singletons pool 2) Storyboard instantiate new ViewController instance and replaces ViewController stored in the pool.

I think it's correct behavior since we haven't viewController from storyboard yet, but we need to inject that into AppDelegate.. I can't see any good and consistent way to do that.

If we do two-step viewController instantiation, then it's possible to get different instances for same singleton definition, which looks weird.

[alexgarbarev]

I think we should raise an exception, while creating ViewController from storyboard and if we have instance in any singletons pool. Something like "We can't create ViewController from storyboard, since it was already instantiated - please use another definition/scope for that one"

[alexgarbarev]

Another possible bug in the Demo project:

- (ViewController *)viewController
{
    return [TyphoonDefinition withClass:[ViewController class] configuration:^(TyphoonDefinition *definition) {
        [definition setScope:TyphoonScopeLazySingleton];
    }];
}

Once ViewController wasn't created by storyboard and stored here, Typhoon will create a 'bare' ViewController without xib counterpart, just alloc-init created object - not good. Maybe we can create something special for storyboards. Maybe another scope.. Or special definition for Storyboard-based viewControllers

[alexgarbarev]

Something like

- (UIViewController *)viewController
{
   return [TyphoonDefinition withStoryboardId:@"ViewController" configuration:^(TyphoonDefinition *definition) {
        /// injections
        [definition setScope:TyphoonScopeLazySingleton]; /// <-- this can be default scope for that kind of ViewControllers
    }];
}

Then initialization would be from Storyboard by specified identifier. ViewController would be created by first call and then cached into Singletons (weak or laze) pool.

If ViewController already exists in pool, we can return it from

- (id)instantiateViewControllerWithIdentifier:(NSString *)identifier

method of TyphoonStoryboard

@igrekde What do you think? Make any sense ?

[etolstoy]

@alexgarbarev that looks great, thanks. Returning the pre-cached instance from - (id)instantiateViewControllerWithIdentifier:(NSString *)identifier is exactly what I meant under two-step instantiation :)

Such definition should use not only view controller identifier, but also storyboard identifier:

- (UIViewController *)viewController
{
   return [TyphoonDefinition withStoryboard:[self myStoryboard] storyboardId:@"ViewController" configuration:^(TyphoonDefinition *definition) {
        /// injections
        [definition setScope:TyphoonScopeLazySingleton]; /// <-- this can be default scope for that kind of ViewControllers
    }];
}

or

- (UIViewController *)viewController
{
   return [TyphoonDefinition withStoryboardName:@"StoryboardName" storyboardId:@"ViewController" configuration:^(TyphoonDefinition *definition) {
        /// injections
        [definition setScope:TyphoonScopeLazySingleton]; /// <-- this can be default scope for that kind of ViewControllers
    }];
}

BTW, currently we use a very same approach, but more verbose:

- (FeedViewController *)viewControllerFeedModule {
    return [TyphoonDefinition withFactory:[self.storyboardAssembly feedStoryboard]
                                 selector:@selector(instantiateViewControllerWithIdentifier:)
                               parameters:^(TyphoonMethod *factoryMethod) {
                                   NSString *identifier = NSStringFromClass([FeedViewController class]);
                                   [factoryMethod injectParameterWith:identifier];
                                } configuration:^(TyphoonFactoryDefinition *definition) {
                                    [definition injectProperty:@selector(output)
                                                          with:[self presenterFeedModule]];
                                }];
}

[alexgarbarev]

@igrekde Yes, I see now. The other reason of another kind of definition is: it's more safe, since it guaranteed Storyboard-based ViewController and has identifier for that. Also we could check inside instantiateViewControllerWithIdentifier, that returning instance was created using Storyboard definition.

That's why I'm thinking of a two-step view controller instantiation:

1. We look for the singleton instance in component pools, and if there is any, we return it.
2. If there are no singleton instances, we return the original one.

We should put into singletons pool in case 2 (if we have definition with lazy singleton scope).

[etolstoy]

@alexgarbarev You're right, I forgot to mention this step.

I'll take time to implement this feature in the a couple of days.

[alexgarbarev]

:+1: Awesome!

[uranpro]

Have singleton bug too

// db manager code

- (id)initWithDbName:(NSString *)dbName {
    self = [super init];
    if (self) {
        NSLog(@"INIT WITH %@", dbName);
        [self setupDatabase:dbName];
        NSLog(@"Db %@", self.db);
    }
    return self;
}

// assembly

- (TQDatabaseManager *)dbManager {
    return [TyphoonDefinition withClass:[TQDatabaseManager class] configuration:^(TyphoonDefinition *definition) {
        [definition useInitializer:@selector(initWithDbName:) parameters:^(TyphoonMethod *initializer) {
            [initializer injectParameterWith:TyphoonConfig(@"db.filename")];
        }];
        definition.scope = TyphoonScopeSingleton; // not working =,(

    }];
}

- (TQOfflineViewController *)offlineViewController {
    return [TyphoonDefinition withClass:[TQOfflineViewController class] configuration:^(TyphoonDefinition *definition) {
        [definition injectProperty:@selector(dbManager) with:[_coreComponents dbManager]];
    }];
}

// main
// before this code DBManager already initialized, see log

- (void)offlineTap {
    TQOfflineViewController *controller = [[[TQApplicationAssembly assembly] activate] offlineViewController];

    [self presentViewController:controller animated:YES completion:^{
    }];
}

// log

2015-11-30 17:11:47.968 TQ[1364:800053] INIT WITH db.sqlite
2015-11-30 17:11:48.179 TQ[1364:800053] Db <YapDatabase: 0x145d080c0>
2015-11-30 17:11:55.920 TQ[1364:800053] INIT WITH db.sqlite
2015-11-30 17:11:55.924 TQ[1364:800053] Db (null)

[etolstoy]

Fixed by #451.