Return to Answer

• 50.1k
• 3
• 130
• 210

• 50.1k
• 3
• 130
• 210

1. This is well documented, well commented code.

2. The docstring says:

The one way in which slicing a WraparoundList differs from slicing an ordinary list is the case of using the list length as the upper limit.

but this isn't quite the whole story — an ordinary list can also be sliced using a value greater than the list length, and in that case WraparoundList also has a different behaviour:

 >>> x = [1, 2, 3]
 >>> x[:10]
 [1, 2, 3]
 >>> x = WraparoundList(x)
 >>> x[:10]
 [1]

1. The code is not portable to Python 3, because there's no sys.maxint (all integers in Python 3 are "long"). I suggest something like this:

    try:
    # In Python 2.7, when __*slice__ methods are called with no "stop"
    # value, sys.maxint is passed instead.
    from sys import maxint as NO_STOP
    except ImportError:
    # Python 3 does not have sys.maxint or use the __*slice__ methods.
    NO_STOP = object()
   

I prefer a name like NO_STOP because it communicates the intention rather than the implementation.

1. _wrap_index raises ZeroDivisionError if the list is empty:

    >>> w = WraparoundList([])
    >>> w[0]
    Traceback (most recent call last):
    File "<stdin>", line 1, in <module>
    File "cr153920.py", line 79, in __getitem__
    return list.__getitem__(self, self._wrap_index(i))
    File "cr153920.py", line 110, in _wrap_index
    return i % _len
    ZeroDivisionError: integer division or modulo by zero
   

Raising an exception is the right thing to do in this case, but I would expect to get an IndexError instead.

1. The code calls list.__getitem__ directly, rather than via the super function. But this has the unsatisfactory consequence that if someone has another class C also inheriting from list and overriding the __getitem__ method, and combines WraparoundList and C via inheritance, like this:

    class D(WraparoundList, C):
    pass
   

Then D()[0] calls WraparoundList.__getitem__, which calls list.__getitem__, but C.__getitem__ is never called, contrary to what one would expect. If you want to support subclassing of WraparoundList, then you need to write:

 return super(WraparoundList, self).__getitem__(self._wrap_slice(i))

and so on.

1. With a little refactoring, you could avoid some of the repetition. In particular, if you had a method like this:

    def _wrap_arg(self, i):
    if isinstance(i, slice):
    return self._wrap_slice(i)
    else:
    return self._wrap_index(i)
   

Then you'd be able to write:

 def __getitem__(self, i):
 """x.__getitem__(i) <=> x[i]"""
 return super(WraparoundList, self).__getitem__(self._wrap_arg(i))

and so on.

1. Once you've done the refactoring above, you'll see that _wrap_index and _wrap_slice are only called from one place each, so they could be inlined at their point of use.

2. There is no need to include an empty main function or an if __name__ == '__main__': section — if there's nothing to do, then there's no need to write code to do it.