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

minor code cleanup #23

Merged
merged 1 commit into from
Jan 4, 2016
Merged

minor code cleanup #23

merged 1 commit into from
Jan 4, 2016

Conversation

phraktle
Copy link
Contributor

@phraktle phraktle commented Jan 4, 2016

make NoCache a singleton

@oschwald
Copy link
Member

oschwald commented Jan 4, 2016

Is there a particular reason for this change? My concern about making it a singleton is that it limits our ability to change the behavior in the future without breaking the API (e.g., if we wanted to make it hold some sort of per-reader state). I do admit it is hard to imagine a reason for such a change, but I am curious is there is a reason for this change.

@phraktle
Copy link
Contributor Author

phraktle commented Jan 4, 2016

I expect NoCache to remain a stateless instance (it's basically just a no-op). I also don't expect usage of this directly (since it's the default anyway). The reason why I made this change was there was one static instance of it in the DB Reader and I had to make another one in the GeoIP API as well. With the singleton, that could be updated to use the same instance. Either way, not a big deal, just thought it would be nicer with a proper singleton.

oschwald added a commit that referenced this pull request Jan 4, 2016
@oschwald oschwald merged commit 382da4d into maxmind:master Jan 4, 2016
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.

2 participants