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

Rest Framework Settings file Performs an O(n) operation on settings access that could be O(1) #3786

Closed
CuriousG102 opened this issue Dec 31, 2015 · 4 comments
Labels
Milestone

Comments

@CuriousG102
Copy link

@CuriousG102 CuriousG102 commented Dec 31, 2015

Calling .keys() on a dictionary returns an iterable, so doing a membership test against that iterable is an O(n) operation. By not including a call to .keys(), the same effect will be attained in O(1) time.

https://github.com/tomchristie/django-rest-framework/blob/af0ea8ef5136da30cb058b10a493d36619d24add/rest_framework/settings.py#L199

@jpadilla
Copy link
Member

@jpadilla jpadilla commented Dec 31, 2015

@CuriousG102 mind submitting a PR to discuss further?

@xordoquy
Copy link
Collaborator

@xordoquy xordoquy commented Dec 31, 2015

Sounds like a smal improvement since values are cached after that but still possible

@CuriousG102
Copy link
Author

@CuriousG102 CuriousG102 commented Dec 31, 2015

Ah. I didn't notice the caching going on on line 214. That does indeed make this a small improvement. Otherwise it might have been a bit bigger depending on how often settings are accessed. In any case I'll make a PR after New Years celebrations.

@xordoquy
Copy link
Collaborator

@xordoquy xordoquy commented Jan 1, 2016

Thanks and enjoy the celebrations !

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

No branches or pull requests

3 participants