Repository navigation
Throwing errors instead of fatalError #15
Description
Probably we should consider throwing errors in resolve instead of using fatalError. For now we use it only in one place, but there could be more places where we will need to signal developer that something is wrong. try enforces developer to pay attention to possible errors. Also with throwing errors we can test those invalid cases what we can't do with fatalError.
Activity
AliSoftware commentedon Nov 25, 2015
Yeah I was wondering about that the first time I added
fatalError.I didn't want to use
throwat that stage because:let ws = container.resolve() as WebServiceAPIand would have to wrap that in ado…catchfatalErrorfeels right.If we don't
do…catchin the case of the first bullet point but usetry!instead, that's kinda acknowledging that failing to resolve should make the app crash and that would be a developer error not a user error, and would result in the same as doingfatalErrorafter all.So that was my reasoning at the time. Maybe moving to
throwcould make sense but if that results to all users usingtry!all the time, not sure that's worth it? I mean, I hate force-unwrapping optionals, so generally I also hate seeingtry!in the user code as well. But maybe we should to make the developer aware of the risks, I'm not sure what I prefer here.ilyapuchka commentedon Nov 25, 2015
Agree that this will make to use try! with every resolve, but that's good for developer, I think, cause it indicates that this is a point where something can go wrong. So developer should think upfront is it safe to use. With fatalError he will notice that he did something wrong only at runtime. Also (just imagining, didn't thought through) there can be use cases for something like plugin systems or something backed up by in-app purchases, when not having a registered component does not mean a fatal error. The other important argument for throwing is that we will be able to test such negative cases in Dip unit test.
AliSoftware commentedon Nov 25, 2015
Ok, you convinced me migrating to
throwand let the user calltry!might be indeed better.Note that as this will break the existing API (new API not retro-compatible) this will lead to a new major version of Dip so might integrate that feature at the same time we remove the deprecated method
register(instance:)and do other major changes to the API if any.ilyapuchka commentedon Nov 25, 2015
Maybe we can have some kind of roadmap that we discuss upfront and have somewhere in this repo which will outline a set of features we plan to release? Also we can use it to discuss do we ever need particular features or not and where will they go - in core or extensions.
AliSoftware commentedon Nov 25, 2015
Yeah we should.
Idk what's best a github issue or a wiki that we could edit live?
I also like to use GitHub milestones to prioritize issues and tell in which version they are plan to go like a RoadMap, and use GitHub labels to tag issues (like we could have a "Core" and "Extensions" labels)
ilyapuchka commentedon Nov 25, 2015
Wiki page will be ok I think
AliSoftware commentedon Nov 25, 2015
Well actually I think about this:
You should have all the necessary access right to start all this (milestones, labels, wiki) on your own if you want (I have to go tonight) otherwise will look at it tomorrow evening.
AliSoftware commentedon Nov 26, 2015
@ilyapuchka I've created this wiki page for a start, and also the two
CoreandDedicated Repolabels. Feel free to create issues for the future features you plan to integrate (like UI extensions, etc) and flag them, and to create GitHub milestones too.