TDD in Python with pytest - Part 2 (average)

[labdmitriy]

Hi Leonardo,

I followed Step 9 and at first tried to implement it myself, compared with your implementation and made some notes:

1. Requirements 4 and 5 have the values of lt (5) and ut (12) which are different from the values in the implementation (2 and 98 accordingly)

2. Your final implementation:

   def avg(self, it, lt=None, ut=None):
       _it = it[:]
   
       if lt is not None:
           _it = [x for x in _it if x >= lt]
   
       if ut is not None:
           _it = [x for x in _it if x <= ut]
   
       if not len(_it):
           return 0
   
       return sum(_it)/len(_it)

   has some properties which probably can be improved:

   • According to the requirements, this method should accept any iterable, but your implementation can't process generators (which are iterators and also iterables) because for example len() function can't be used with generators.
   • Iterable is copied and additional memory is required
   • 4 passes through the iterable are required for your implementation (copy, 2 list comprehensions and sum)

I added test for generators at first:

def test_avg_accepts_generators():
    calculator = SimpleCalculator()
    result = calculator.avg(i for i in [2, 5, 12, 98])
    assert result == 29.25

Then I implemented the method which also accepts generators, does not copy iterable and requires only 1 pass:

 def avg(self, it, lt=None, ut=None):
        count = 0
        total = 0

        for number in it:
            if lt is not None and number < lt:
                continue
            if ut is not None and number > ut:
                continue
            count += 1
            total += number

        if count == 0:
            return 0
        else:
            return total / count

Thank you.

[lgiordani]

Thanks again!

1. Yes, a mistake!
2. Very interesting! I will review your solution and add your consideration (if you don't mind, clearly mentioning you) as I believe they are a good example of further requirements that can be tackled with a refactoring.

Amazing!

[lgiordani]

@labdmitriy apologies for the long delay with this issue. I'm working on it at the moment. I reviewed point one, which I though was a mistake. I double checked and it makes sense to me.

Looking at point 4, I see that the example shows that avg([2, 5, 12, 98], ut=12) == avg([2, 5, 12]), that is the value of ut should be included in the filtered list.

So, just to double check, is your point that the implementation of the test is different from the example? Thanks!

[lgiordani]

@labdmitriy I added your implementation to the project. I also briefly discussed performances and style. Thanks!