-
Notifications
You must be signed in to change notification settings - Fork 268
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
If initializer arguments not equal to number in selector, then fire warning #97
Comments
+1 (A nice runtime error message would be good here, yes, but this is also one of the situations where better documentation would help.) |
@ratkins - You say error message. . I was actually thinking of just making it a warning. Do you think that would be ok? This still allows you to implicitly inject nil as a parameter argument, but just not providing one. Also, yes. . The documentation needs a lot of attention. . So far only folks who are familiar with DI will be able to use the project. . Ideally I want folks who've never tried DI to be able to jump straight in. |
Weighing the "ugliness" of forcing you to specify a nil parameter if that's what you want vs the inscrutability of things that intuitively ought to work, not working, I'd favour making it an error. |
@ratkins OK, error it is then. In fact the way to inject a 'nil' is quite ugly and not so intuitive (you inject an 'objectInstance' of nil). But we can add instructions in the error message. This way, whether you were imagining 'by type' injection or 'a nil', Typhoon will stop and lay out the deal for you. |
Fixed. . @ratkins, thanks for reporting. |
The objective-c run-time does not allow Typhoon to have initializer auto-injection by type.
However, not knowing this, it would feel natural to specify an initializer, and expect Typhoon to auto-wire. (Let's of users have tried this).
In this case Typhoon should fire a warning:
"Number of provided arguments does not match the number in the initializer. Note that Typhoon is not able to provide by-type injection for initializers. Consider using a property, if this is required"
@ratkins - this one's for you.
The text was updated successfully, but these errors were encountered: