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

Methods to not cause exception on misses. #12

Closed
wants to merge 4 commits into from
Closed

Methods to not cause exception on misses. #12

wants to merge 4 commits into from

Conversation

mmicco-godaddy
Copy link

I wanted to have the ability when using database to NOT throw an AddressNotFound exception. In this pull request I implemented this in the following way:

  1. I only made the change for the DatabaseReader. I am assuming the cost of the exception is less important if calling webservices, as they would have more overhead to begin with.
  2. Instead of implementing a change to the existing methods that do a get, I made two new methods for city and country named "TryGet" that follow .NET's standard TryGet methodology.
  3. If these changes are acceptable, I will also implement TryGet's for the ISP, connection type and domain lookups before a final pull request.

Additional question: is there any versioning or assembly info changes I need to make for a pull or will that just be taken care of by the next release? (given my pull is accepted)

@oschwald
Copy link
Member

I apologize for not responding to your question in #11 previously. Our databases should have records for almost all public, allocated IP addresses. As such, I am not sure I understand the use case where exception handling is a significant burden. Generally, if you are getting a significant number of exceptions, that suggests that there is an integration problem, such as using the private IP of load balancer rather than the public IP of the end user.

@mmicco-godaddy
Copy link
Author

Cool. I admit I did not do any comprehensive test to see what the miss % would be even if I ignored any internal ip addresses. I probably will measure that during some performance testing.

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

Successfully merging this pull request may close these issues.

None yet

2 participants