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

Stop throwing AddressNotFoundException when an address can't be found. #31

Closed
niemyjski opened this issue Jul 15, 2015 · 4 comments
Closed

Comments

@niemyjski
Copy link

It might sound like a good idea to throw an AddressNotFoundException when a lookup can't be found but you should favor returning null or doing something else. Throwing one exception is fine but if you use this library in a high throughput system, there are performance issues you will run into....

"As MusiGenesis said, "Exceptions are slow in .NET if they are thrown"

So, yes, you don't want to throw too many of them. On my computer, a simple throw inside a loop, it took 9.5 seconds to throw 1000 exceptions, but that time would increase, were the exception to be buried inside a large stack.

That is why flow control should NOT be done by exceptions (I know your question didn't ask for anything other than the speed, but you can work that out yourself with a timed test). Sure, you should throw exceptions whenever something goes wrong. However, you should keep the number of things going wrong to a minimum. E.g. don't pass bad data to a function.

Ha, and every answer so far (including this) is talking about when to, and when not to throw exceptions. Ironic." http://stackoverflow.com/questions/161942/how-slow-are-net-exceptions

@oschwald
Copy link
Member

This is part of the API and is unlikely to be changed before a new major release, which may never happen. As discussed in #12, if you are getting many of these exceptions, you likely have an integration issue.

That said, we might consider a PR for corresponding Try... methods in the spirit of TryParse.

@manigandham
Copy link

Agree with this, exceptions shouldn't be used for flow control.

There are several reasons why the IP might not be valid and if there's no information then it's just a null or lack-of-info response. Even maxmind states that not every IP is included in the database which means there are valid IPs that still throw an error which doesn't make sense.

I'll submit a PR for this.

@oschwald
Copy link
Member

Closed by #50.

@niemyjski
Copy link
Author

Thank you!

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

No branches or pull requests

3 participants