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

Initializers returning nil cause Typhoon to crash #314

Closed
rodgutierrez opened this issue Jan 28, 2015 · 3 comments
Closed

Initializers returning nil cause Typhoon to crash #314

rodgutierrez opened this issue Jan 28, 2015 · 3 comments

Comments

@rodgutierrez
Copy link

I'm using Typhoon to inject an instance of a class that is only supported on certain versions of iOS (it's a wrapper for HealthKit functionality). When this class is instantiated on unsupported versions of iOS its initializer returns nil, which appears to be acceptable behavior. However this pattern is causing Typhoon to crash.

This crash is happening while running Typhoon 2.3.1 in TyphoonComponentFactory on line 379. Here's the error:

2015-01-27 18:08:33.702 Strava[17099:607] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '*** setObjectForKey: object cannot be nil (key: healthStoreService)'
@jasperblues
Copy link
Member

If a valid class has an initializer that returns nil, the question is:

  • Should Typhoon fail, as it does now, but with a more meaningful error message, and make the user work-around this?
  • Should Typhoon accept this and simply issue a warning that no instance will be injected, since the instance is nil?

Let's try to enumerate cases where:

  • the first scenario has benefits that outweigh the 2nd.
  • the second scenario has drawbacks that outweigh the first.

If we can't see any obstacles, let's push 2.3.5 with a fix to this within 24 hours. In this case would you like to send a PR? (Simply wrap the injectInstance:withDefinition method in a nil-check)

@jasperblues
Copy link
Member

@rodgutierrez I've added the ability for an instance returned from TyphoonComponentFactory+InstanceBuilder to validly be nil. (Actually good timing,k it turns out we needed this today too :) )

To try:

pod 'Typhoon', :head

Once you verify we'll push a new micro version to CocoaPods master.

@rodgutierrez
Copy link
Author

@jasperblues This looks great! 👍

Thank you for the quick response.

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