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

Allow easier definition of additional type converters (ie. in the Assembly). #21

Closed
jasperblues opened this issue May 24, 2013 · 3 comments

Comments

@jasperblues
Copy link
Member

Typhoon allows the definition of additional type converters like this:

TyphoonComponentFactory* factory = [TyphoonBlockComponentFactory factoryWithAssembly:[ApplicationAssembly assembly]];
[[TyphoonTypeConverterRegistry shared] register:[[HistoryModeTypeConverter alloc] init] forClassOrProtocol:[HistoryMode class]];

It might be nice to allow defining type converters in the assembly itself.

@eriksundin
Copy link
Contributor

Suggested solution:

Alter the TyphoonTypeConverter protocol to provide the supportedType.

@protocol TyphoonTypeConverter <NSObject>

- (id)supportedType;

- (id)convert:(NSString*)stringValue;

@end

The TyphoonTypeConverterRegistry now only takes a TypeConverter like so:

/**
* Adds a converter to the registry.
*/
- (void)register:(id <TyphoonTypeConverter>)converter;

Create a PostProcessor that registers new type converters automatically. The post processor instantiates the type converter and adds it to the registry. It needs to do a check if the converter already exists since the PostProcessor could be called multiple times.

Discussion: The existing converter check will omit the exception of trying to add two converters for the same type, this has the effect that if the user has has declared two converters in the assembly for the same type it will accept this silently. Could be acceptable?

@implementation TyphoonTypeConverterRegistrar
{
    TyphoonTypeConverterRegistry* _registry;
}

- (id)initWithTypeConverterRegistry:(TyphoonTypeConverterRegistry*)registry
{
    self = [super init];
    if (self)
    {
        _registry = registry;
    }
    return self;
}

- (id)init
{
    return [self initWithTypeConverterRegistry:[TyphoonTypeConverterRegistry shared]];
}

- (void)postProcessComponentFactory:(TyphoonComponentFactory*)factory
{
    for (TyphoonDefinition* definition in [factory registry])
    {
        if ([definition.type conformsToProtocol:@protocol(TyphoonTypeConverter)])
        {
            id <TyphoonTypeConverter> converter = [factory buildInstanceWithDefinition:definition];
            TyphoonTypeDescriptor* descriptor = [TyphoonTypeDescriptor descriptorWithClassOrProtocol:[converter supportedType]];
            if ([_registry converterFor:descriptor] == nil)
            {
                [_registry register:converter];
            }
        }
    }
}

@end
  • Appreciate any comments or feedback on possible problems with this solution.
  • Adding this PostProcessor by default to the ComponentFactory could also be considered. Removing the need to declare it the assembly.
  • The name TyphoonTypeConverterRegistrar might be too close to TyphoonTypeConverterRegistry. Other name suggestions appreciated. ;-)

@jasperblues
Copy link
Member Author

This looks great - I don't see any problems.

I think its worth loading this post-processor by default. . . We should check to ensure that post-processors go away once they've done their job - to save memory.

I do agree that the name might lead to confusion. . . perhaps just something simple like TyphoonTypeConverterRegistrationPostProcessor ? (geez - starting to sound like a Spring class - so long and descriptive. . . gets the job done though ;) )

@eriksundin
Copy link
Contributor

Thanks Jasper. I will move forward with the above solution and look into adding some post processor clean up once applied.

Haha. Agree on the long name, very Springish.

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

2 participants