Recursion
Recursion is a bad idea if a simple loop does the job and if you have little control over recursion depth.
In [1]: import sys
In [2]: sys.getrecursionlimit()
Out[2]: 1000
Of course you can set a higher limit, but you may run into a stack overflow. But lets refactor your code first.
Code repetition
Code repetition is one of the ugliest things you can do. copy pasted code is a real pitfall when you forget o update the other execution paths. We change
def username_system(u, memo={}, users=[]):
copy_u = u.copy()
try:
name = copy_u[0]
except IndexError:
return users
if name in memo.keys():
memo[name] += 1
username = name + str(memo[name])
users.append(username)
copy_u.remove(name)
return username_system(copy_u, memo, users)
else:
username = name
users.append(username)
memo.update({name: 0})
copy_u.remove(name)
return username_system(copy_u, memo, users)
return users
to
def username_system(u, memo={}, users=[]):
copy_u = u.copy()
try:
name = copy_u[0]
except IndexError:
return users
if name in memo.keys():
memo[name] += 1
username = name + str(memo[name])
else:
memo.update({name: 0})
username = name
users.append(username)
copy_u.remove(name)
return username_system(copy_u, memo, users)
return users
We immediately see the unreachable code at the end and remove the last line
Try/catch instead of if/else
You misuse exception handling for a simple test. We replace
try:
name = copy_u[0]
except IndexError:
return users
by
if len(copy_u) == 0:
return users
name = copy_u[0]
Avoid unnecessary copies
Where you iterate over your list you do
copy_u = u.copy()
for no reason. This your absolute performance killer as it is of quadratic complexity. We can delete that line and the code is still working. If we want to save the initial list we do
print(username_system(names.copy()))
in our main function.
Be careful about remove()
list.remove()
searches(!) for a value and deletes it from the list. You already know which index to delete, so use del
.
In your case remove()
has no negative impact to complexity as the element is found immediately at the front. However the code is more readable when you use del
as this tells everybody that no search is done.
Current status
def username_system(u, memo={}, users=[]):
if len(u) == 0:
return users
name = u[0]
if name in memo.keys():
memo[name] += 1
username = name + str(memo[name])
else:
memo.update({name: 0})
username = name
users.append(username)
del u[0]
return username_system(u, memo, users)
And now the subtle bug
If you call your function multiple times there is some persistence
print("given:", names)
print("returns:", username_system(names.copy()))
print("given:", names)
print("returns:", username_system(names.copy()))
prints
given: ['john', 'john', 'tom', 'john']
returns: ['john', 'john1', 'tom', 'john2']
given: ['john', 'john', 'tom', 'john']
returns: ['john', 'john1', 'tom', 'john2', 'john3', 'john4', 'tom1', 'john5']
How is that? You do default params in your function. This default value is created only once. If you alter the value, which is possible on containers like list
the altered value persists. When you call your function the second time memo
and users
are initialized to the previously used objects and continue the up-count. That can be solved like
def username_system(u, memo=None, users=None):
memo = memo or {}
users = users or []
Some other python stuff
if name in memo.keys():
can be replaced by
if name in memo:
The default iteration over a dict()
gives the keys. Use dict.keys
only if you e. g. want to copy keys to a list().
In the module collections
there is a class Counter
which does exactly what your memo
does. We use it like
from collections import Counter
def username_system(u, memo=None, users=None):
memo = memo or Counter()
users = users or []
# [...]
if name in memo:
username = name + str(memo[name])
else:
username = name
memo[name] += 1