-
Notifications
You must be signed in to change notification settings - Fork 24
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
Find should return null when item isn't found, instead of throwing an exception #52
base: master
Are you sure you want to change the base?
Conversation
Should return null when item not found instead of throwing
Well, it depends, actually :) E.g. Entity Framework does return null, yes. While ADO.Net throws. Because of that ambiguity and because this definitely looks like a breaking change, I would propose to introduce separate methods (e.g. TryFind() and TryFindAsync()) instead. |
Apologies if I'm looking at the wrong place, but from what I'm reading, DataViews in ADO.NET don't throw, they also return null or -1 for rows that aren't found, my source is here: https://msdn.microsoft.com/en-us/library/yth8t382(v=vs.100).aspx can you let me know which ADO.NET construct you're referring to if I'm lokoing at the wrong one? |
Sorry, you included the link to your source in your comment :P however, in your link, it says:
However I think you're looking at the part that says
if you look at the source for the method, it looks like this
Following that code through, it looks like the only time it actually throws that exception is when the key is null or empty, rather than when the node isn't found. You can replicate my rabbit hole here |
Anyway, I would prefer not to do a breaking change and introduce separate methods with that new behavior. Would you agree? |
I certainly understand the desire not to introduce a breaking change for this, that being said though it would preferably only be temporary until we DO make that change to eliminate technical debt and bring it in line with the framework conventions underlying it. In my opinion I'd rather make the breaking change and advise the community about it rather than creating a stop-gap, but interested in your suggestions |
Hi Konstantin, do you have a final decision on this? if you dont want to implement it this way, feel free to tweak it as you see fit, but we should get it closed out when we can. Thanks! |
Hi, sorry, I was having a vacation :) I still think, that a new separate method is better. Don't really like breaking changes, particularly without clear reasons. |
Personally I think the reason is clear; the typical convention for .NET is for Find to return null, not an exception. However, it's your project, so if you want to change it then feel free, just want to get this closed out |
Refer to #51