- edited description
Purpose of checking current instance in every iteration of while loop?
If I understand correctly, these 2 lines make sure that the current instance is not already in the database.
if instance.pk:
rivals = rivals.exclude(pk=instance.pk)
Why don't we move this check before the while
loop and return if instance.pk
is not None
(a valid instance.pk
implies it's not a new object but an update operation) and hence there's no need to re-generate a unique slug. The while
loop can be skipped in that case.
Comments (3)
-
reporter -
repo owner Thanks for reading the code so thoroughly. The presence of a PK does not necessarily guarantee that the slug should not be regenerated because the user/app could reset it to an empty string or, even worse, replace it with another value which is already in the database. Still, there may be room for improvement. For example, we could track changes and only enter the
while
loop if either the PK is missing or the value of the field has been changed.By the way, I don't have much time for this project these days, so the best way to push an improvement into the trunk is to create a pull request with comprehensive tests (and documentation if needed). It's also preferable that the contributor adds his/her full name to the AUTHORS file.
-
reporter - changed status to resolved
Thank you for the clarification.
- Log in to comment