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

Factory provider #101

Merged
merged 3 commits into from
Nov 12, 2013
Merged

Conversation

drodriguez
Copy link
Contributor

TyphoonFactoryProvider implements a kind of automatic creation of factory classes for the user code. The user only has to provide a protocol with the interface of the factory she wants to create, and some “glue” code, and the factory will be created at runtime for her.

The idea for the TyphoonFactoryProvider comes from Guice AssistedInject. Since the ObjC runtime is a little less typed than Java’s, some of the “automagic” that Guice guys manage to pull off can’t be reproduced in Typhoon. The dependencies has to be wired by hand, and the body of the factory methods have to be provided explicitly.

Even with those drawbacks, the code the user has to write is reduced by a lot of lines (no need to write the factory initializer, no need to declare the storage for the dependencies, most of the implementation block…), so I think is still useful.

There is a little “example” written as documentation for the TyphoonFactoryProvider class, and the tests use the same example.

I also fixed some problems I found while compiling.

Known limitation: for each protocol you only can have one factory “implementation”. If you invoke TyphoonFactoryProvider twice for a protocol the “implementation” will be the first one to run at runtime. This limitation can be easily removed, but I don’t know if the runtime will create one new class per definition or one per use of the definition.

PD: I was not able to run the test in iOS, I suppose they are not broken, but…

objc_sendMsg is not in obj/runtime.h but in objc/message.h.
TyphoonFactoryProvider simplifies the task of creating factories for
your classes, removing a lot of boilerplate code and typing.
@jasperblues
Copy link
Member

Nice work Daniel

. . . I started implementing this where the parameters would get patched in a meta-data driven way (instead of providing a block as you've done) however I wasn't able to finish as I didn't have the time to invest in such a complicated feature.

  • I think in future it might be possible to provide an automatic implementation of that patching block using libffi

Daniel, would you be able to help us out and send a pull request against the latest Typhoon? There have been a few changes in the past few days and this one won't merge automatically.

@jasperblues
Copy link
Member

Oops. . as it turns out there was only one small conflict preventing this from auto-merging. . So I'll go ahead and merge now.

jasperblues pushed a commit that referenced this pull request Nov 12, 2013
@jasperblues jasperblues merged commit 229b6a4 into appsquickly:master Nov 12, 2013
@jasperblues
Copy link
Member

Merged - thanks again Daniel!

Would you like to contribute to the user guide regarding this feature?

@drodriguez
Copy link
Contributor Author

Thanks for merging. I will write a wiki page in the next hours.

About the rest of the comments: I was stupid not to look into the issues before. It’s obvious someone must have though the same. I hope I haven’t stepped in someone else work too much. I will review those threads and see if I have covered those requirements.

I though about the “metadata” driven approach, but I think the user classes should not being tied to Typhoon, or at least, the metadata approach should be the alternative approach. I still think you have to provide a lot of code for simple factories, and I will try to reduce it. I think that for simple factories/init, it should be possible. I will have a look at libffi, but I think there is still the problem of matching arguments from the factory method and the constructor (if only ObjC didn’t erase all the type information for object arguments…).

@drodriguez
Copy link
Contributor Author

I have written an user guide at my wiki. I think you can access the raw Markdown to copy into the official wiki using https://raw.github.com/wiki/drodriguez/Typhoon/Factory-providers.md (I haven’t found a way to do a pull request to the wiki repository :()

@jasperblues
Copy link
Member

Hi Daniel,

Looks great. I've given you push access so you can go ahead and add it in. (Or I you don't want this I can do it?).

The general rules for push access are:

  • High impact or experimental features should be done on a branch and discussed before merging.
  • Simple features and bug-fixes should be committed to master - to reduce merge headaches, and to push out new micro versions as often as possible.
  • No obligation to commit new features, but the option is there if you need it. . . I think I can handle up to 5 active commiters. . . If you haven't committed for a while, and someone else has been busy I might reassign access to them. . . This doesn't mean your work wasn't HUGELY appreciated, just a reflection of my ability to multi-task ;)

@drodriguez
Copy link
Contributor Author

Done. Thanks for the push access.

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

Successfully merging this pull request may close these issues.

2 participants