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

Try based lookup methods #50

Merged
merged 6 commits into from
Feb 10, 2016
Merged

Try based lookup methods #50

merged 6 commits into from
Feb 10, 2016

Conversation

manigandham
Copy link

Refers to #31

Added new TryCity and TryCountry methods that don't throw an exception when the IP address cannot be found. This avoids using exceptions for flow control.

@manigandham
Copy link
Author

I can add Try... methods to the other lookup types as well if wanted.

@oschwald
Copy link
Member

oschwald commented Feb 1, 2016

Thanks! Please do add them for all the database types along with corresponding unit tests.

@manigandham
Copy link
Author

Updated.

/// <param name="ipAddress">The IP address.</param>
/// <param name="response">The <see cref="AnonymousIPResponse" />.</param>
/// <returns>A <see cref="bool" /> describing whether the IP address was found.</returns>
public bool TryAnonymousIP(string ipAddress, out AnonymousIPResponse response)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some of these, there is only a method that takes a string IP and for others the method takes only an IPAddress. We should have both or at least be consistent.

@manigandham
Copy link
Author

All set now. You're right, I missed some of the methods in that class.

Added the missing method overloads, organized the class better so the base methods are at the end and refactored the database reader unit tests too.

@oschwald
Copy link
Member

Thanks! I merged this and did a beta release.

@manigandham manigandham deleted the try-based-lookup branch February 10, 2016 19:26
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