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

Implementing xrange

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

Problem

I wanted to learn about Iterable and Iterators and hence thought about getting my hands dirty rather than reading theory and forgetting it.

I am curious how I can improve readability and error handling of my code. I also thought to practice and include test cases.

class foo_range(object):
    """ Custom range iterator which mimics native xrange. """

    def __init__(self, start, stop, step=1):
        try:
            self.current = int(start)
            self.limit = int(stop)
            self.step = int(step)
        except ValueError:
            raise
        if step == 0:
            raise ValueError("Step can't be 0")

    def __iter__(self):
        return self

    def next(self):
        if self.step > 0 and self.current >= self.limit:
            raise StopIteration()
        if self.step < 0 and self.current <= self.limit:
            raise StopIteration()
        oldvalue = self.current
        self.current += self.step
        return oldvalue

import unittest

class TestXRange(unittest.TestCase):

  def test_invalid_range(self):
    with self.assertRaises(ValueError) as ve:
      foo_range(0, 5, 0)

  def test_valid_range(self):
    expected = [0, 1, 2]
    actual = []
    for _ in foo_range(0, 3):
        actual.append(_)
    self.assertEqual(actual, expected)

    expected = [0, -1, -2, -3]
    actual = []
    for _ in foo_range(0, -4, -1):
        actual.append(_)
    self.assertEqual(actual, expected)

    expected = []
    actual = []
    for _ in foo_range(0, 5, -1):
        actual.append(_)
    self.assertEqual(actual, expected)

    expected = []
    actual = []
    for _ in foo_range(0, -5, 1):
        actual.append(_)
    self.assertEqual(actual, expected)

if __name__ == '__main__':
    unittest.main()

Solution

Question

I am not sure to understand the point of having this :

except ValueError:
        raise


List comprehension

You have the following pattern :

actual = []
for _ in foo_range(XXX):
    actual.append(_)


in a few places.

It can easily be replaced by :

actual = [_ for _ in foo_range(XXX)].


Also, _ is usually used as a variable name for throw-away values (values that will not be used). In your case, because you actually use the value, it might be better to name it (a, e, val, i, etc).

Also, I think this is equivalent to :

actual = list(foo_range(XXX))


and I'll let you consider this.

Unit test organisation

Your def test_valid_range(self): test tests multiple things. It is better to split this into multiple smaller tests. Among other things, it is easier to understand if something goes wrong but it also helps you to think about what you are actually trying to test (iterating forward, iterating backward, etc).

Suggestion

It may be interesting to try to implement this using a generator defined with yield.

Code Snippets

except ValueError:
        raise
actual = []
for _ in foo_range(XXX):
    actual.append(_)
actual = [_ for _ in foo_range(XXX)].
actual = list(foo_range(XXX))

Context

StackExchange Code Review Q#107639, answer score: 6

Revisions (0)

No revisions yet.