HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Reversing the elements in the list

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
listthereversingelements

Problem

Q1. Define a function reverse_list that takes a list as an argument
and returns None, but reverses the elements in the list as a side
effect. Do not use any built-in list methods (especially not reverse),
but assignment statements are fine.

Below is the iterative solution:

def reverse_list_iter(s):
    """Reverse the contents of the list s and return None.

    >>> digits = [6, 2, 9, 5, 1, 4, 1, 3]
    >>> reverse_list_iter(digits)
    >>> digits
    [3, 1, 4, 1, 5, 9, 2, 6]
    >>> d = digits
    >>> reverse_list_iter(d)
    >>> digits
    [6, 2, 9, 5, 1, 4, 1, 3]
    """
    median = len(s)//2
    length = len(s)
    for index in range(median):
        temp = s[index]
        s[index] = s[(length - 1) - index]
        s[(length - 1) - index] = temp


Below is the recursive solution:

def reverse_list_recur(s):
    """Reverse the contents of the list s and return None.

    >>> digits = [6, 2, 9, 5, 1, 4, 1, 3]
    >>> reverse_list_recur(digits)
    >>> digits
    [3, 1, 4, 1, 5, 9, 2, 6]
    >>> d = digits
    >>> reverse_list_recur(d)
    >>> digits
    [6, 2, 9, 5, 1, 4, 1, 3]
    """
    median = len(s)//2
    length = len(s)
    def reverse(index):
        if index == median:
            return
        else:
            temp = s[index]
            s[index] = s[(length - 1) - index]
            s[(length - 1) - index] = temp
            reverse(index + 1)
    reverse(0)


How can I improve the solution?

Solution

In general, Python's tuple assignment and unpacking allows you to rewrite:

temp = foo
foo = bar
bar = temp


as:

foo, bar = bar, foo


which saves you two lines in both versions! This gets you to:

s[index], s[(length - 1) - index] = s[(length - 1) - index], s[index]


Rather than indexing by (length - 1) - index, note that e.g. s[-1] is the last item in the list, s[-2] the penultimate item, and so on. So you can rewrite:

s[(length - 1) - index]


as:

s[-index - 1]


and we're now down to:

s[index], s[-index - 1] = s[-index - 1], s[index]


and there's no need to calculate length.

For the iterative version I would factor out median, too, going from:

median = len(s)//2
for index in range(median):
    ...


to simply:

for index in range(len(s) // 2):  # note whitespace
    ...


Although it's now factored out, note that:

median = len(s)//2
length = len(s)


could be rearranged to:

length = len(s)
median = length // 2  # and here


len(s) is O(1), but why duplicate it? DRY!

As I've previously shown, adding an extra parameter with a default value (def reverse_list_recur(s, index=0):) can simplify recursive approaches, removing the need for a nested function.

I would also remove the closure on median and explicit return/else and just test if index < len(s) // 2: before the swap:

def reverse_list_recur(s, index=0):
    """Reverse the contents of the list s and return None.

    >>> digits = [6, 2, 9, 5, 1, 4, 1, 3]
    >>> reverse_list_recur(digits)
    >>> digits
    [3, 1, 4, 1, 5, 9, 2, 6]
    >>> d = digits
    >>> reverse_list_recur(d)
    >>> digits
    [6, 2, 9, 5, 1, 4, 1, 3]
    """
    if index < len(s) // 2:
        s[index], s[-index - 1] = s[-index - 1], s[index]
        reverse_list_recur(s, index+1)


Depending on how you interpret "built-in list methods", the following may be OK:

def reverse_list(s):
    s[:] = s[::-1]

Code Snippets

temp = foo
foo = bar
bar = temp
foo, bar = bar, foo
s[index], s[(length - 1) - index] = s[(length - 1) - index], s[index]
s[(length - 1) - index]
s[-index - 1]

Context

StackExchange Code Review Q#90892, answer score: 6

Revisions (0)

No revisions yet.