Factory provider #114
Factory provider #114
Conversation
Modify TyphoonFactoryProvider to delegate the actual work of creating the method from the protocol to TyphoonAssistedFactoryMethodCreator. TyphoonAssistedFactoryDefinition stores subclasses of TyphoonAssistedFactoryMethod instead of the previous array pair. The block are now stored in TyphoonAssistedFactoryMethodBlock, where the actual work is done. Similarly, TyphoonAssistedFactoryMethodInitializer holds the new code to configure the mapping between the factory method and the initializer. A lot of little classes help (TAFInjectedParameter, TAFParameterInjectedWithProperty, TAFParameterInjectedWithArgumentIndex...). The TAFMethodInitializerCreator is currently unimplemented. (appsquickly#102)
TAFMethodInitializerCreator will be split in several smaller files. InitializerCreator creates TAFMethodClosures and stores them in a static global NSDictionary. The closures need to live while the methods can be called and could be a memory "leak" if the user decides to remove those methods or entire classes from the runtime. Not very usual, so I think the solution is "good enough". TAFMethodClousure is where the work is done. The class describes both the factory method and the initializer to libffi, and creates a closure that ties them together. Many lines are taken and slightly adapted from Michael Ash amazing MABlockClosure. There is a small "proof of concept" test for the InitializerCreator. Fix a copy/paste bug in the TAFMethodCreator convenience method. TAFMethodBlockCreator and TAFMethodInitializerCreator share the code to look for a ObjC method description. Return the TAFMethodInitializer parameter descriptions sorted according to their parameterIndex. Update Pods. TODO: lot of error checking TODO: lot of commenting (appsquickly#102)
Create separate file for TyphoonAssistedFactoryMethodClosure. Validation of initializer and parameters during the construction of the closure. Should catch some typing mistakes. Use parameterIndex instead of the iteration index for filling the arguments of the closure. Fix a leak in typesWithEncodingString:count:. Use allocate: instead of calloc. Use the factory method name instead of the selector name to create the keys for the closure cache. You can have several factory methods calling the same initializer, but only one factory method definition. Added more complete TyphoonAssistedFactoryMethodClosureTest, testing invocation with parameters and properties more or less in isolation. TyphoonFactoryProviderTest duplicates the protocols before using them, so the factory implementations are different for blocks and initializers. There is a new test that does the same as the blocks one, but with initializers. (appsquickly#102) Conflicts: Pods/Pods.xcodeproj/project.pbxproj
A lot of code that used to be in TyphoonFactoryProvider is now in TyphoonAssistedFactoryCreator and its subclasses (one for one factory block, one for a definition block, and one for the future implicit with a result type). The subclasses are supposed to generate a block that returns a TyphoonAssistedFactoryDefinition, which the super class will use in case the class doesn't exists yet (using a block to not instantiate the definition eagerly). TODO: the implicit creation strategy. (appsquickly#102) Conflicts: Pods/Pods.xcodeproj/project.pbxproj
Recover libxml headers in OS X target. While including libffi as Cocoapod I must have forgot to include this back in the OS X target (it is there in the iOS target). A couple of imports to avoid warnings and to correctly compile in iOS. Marking the temporal allocations array of the closure as unused and with precise lifetime. The unused is necessary to avoid a warning, the precise lifetime might not be necessary, but I fear the optimizer might optimize the variable away and break everything. (appsquickly#102)
The method GuessFactoryMethodForProtocol moves from the OneFactory to the base class, since the implicit creator also needs it. The _protocol ivar moves from the @Private to @Protected in the Private.h header, so the implicit creator can access it. (appsquickly#102)
Added a bunch of bogus init methods to the PaymentImpl, to better test the guessing of the initializer for the factory method. (appsquickly#102)
Also some minor renaming and some declaration movement.
Not depending on libffi and depending on Apple forwarding mechanism assure us that they are not going to break it badly suddenly. The code is also a lot simpler. (appsquickly#102)
|
Thanks Daniel! Great work! |
|
Would you like to update the docs? . . is there a way we could provide an example of this in the Typhoon sample app? |
|
What do we mean by: TyphoonAssistedFactoryCreatorManyFactories ? . . . . Ah, I see what it means. . . Can we merge the creator for single factory methods or multiple factory methods onto a single TyphoonAssistedFactoryCreator class? |
|
Ah, excuse. I can see that these two classes remain internal, so the name is not too important. |
|
Yeah, I wasn’t very inspired with the names. And I was starting to feel bad with those long names. There are three creators, each of them related to the three ways of creating the factory: for the method that receives just an implementation block I use I will update the docs ASAP and I will have a look to the sample app to see where can we apply the factory provider. |
|
At first I thought these classes were part of the external API. . . but now I see its just the TyphoonFactoryProvider . . therefore, I'm not at all worried about the long, descriptive names. . |
|
You've done a lot of work. . .if you want to delegate some of the remaining work (docs & sample app), let us know. |
See #102 for more information.
I think that is ready to merge after someone gives it a good look.
I will improve the wiki page after this is merged.