When feasible, use specific names. Your function and variable names
are generic. Help your reader out by selecting more communicative names. A second problem is that the current names are
easily misread: array1
and array2
look similar when scanned
quickly, and nothing intuitive links i
to array1
and j
to array2
. Even in a fully generic context, one can
do much better. For example, the following names are more
compact, less easily confused, and more meaningful because
single values are linked to their collection.
for x in xs:
for y in ys:
...
Use sets to determine what should be removed [but see below]. Selecting appropriate
data structures is one of the keys to successful programming. Before
writing code, put some real thought behind each collection you need:
should it be a tuple, list, dict, or set? Good choices tend to simplify
coding logic greatly. By switching to sets in this example, most
of the need for our own algorithm disappears.
Reconsider your function's design. There are many advantages that flow from
building programs out of functions that don't modify their inputs. Do you
really need to modify the original list, or would it be fine to return a new
list? If possible, opt for the latter. A second drawback for your current
approach is its co-mingling of mutation and return values. In Python, a close
analogue for your function is list.remove()
, which returns None
. It could
have returned true/false, but the language designers opted against that -- an
approach often taken in Python. These considerations don't lead to hard and
fast rules, but in the abstract I would favor a no-mutation approach.
# Mutation.
def remove_all(xs, discard):
removed = set(xs).intersection(set(discard))
xs[:] = [x for x in xs if x not in removed]
return list(removed)
# No mutation.
def remove_all(xs, discard):
removed = set(xs).intersection(set(discard))
return (
[x for x in xs if x not in removed],
list(removed),
)
Addendum: the code above does not do exactly what your code does.
The set logic eliminates duplicates from removed
. If you don't
want to do that, you can adjust things along these lines.
# No mutation.
def remove_all(xs, discard):
discard = set(discard)
return (
[x for x in xs if x not in discard],
[x for x in xs if x in discard],
)
set
here? \$\endgroup\$