Implementation list vs yield #1
Conversation
(fizzbuzz) [bbelderb@macbook pycones2016 (vio_bob)]$ python2.7 -m timeit -s "from fizzbuzz import fizzbuzz_yield" "fizzbuzz_yield()" 1000000 loops, best of 3: 0.522 usec per loop (fizzbuzz) [bbelderb@macbook pycones2016 (vio_bob)]$ python2.7 -m timeit -s "from fizzbuzz import fizzbuzz" "fizzbuzz()" 10000 loops, best of 3: 35 usec per loop
| 5: "Buzz", | ||
| 15: "FizzBuzz", | ||
| } | ||
|
|
||
| with describe('Fizzbuzz'): |
There was a problem hiding this comment.
Hello and thanks for the interest :-)
The problem that I personally see to this kind of test, it's that you don't see which is the business logic applied, which are the requirements being solved: why 3 is "Fizz", why 5 is "Buzz"? The tests should be a main part of your documentation.
IMHO, this kind of tests (something like a parametrized tests) might be good for a final stage, where you don't want to have 100 tests, one for each use case: once you have already discovered and solved the business logic, then it might be good to have a kind of "overall" test to complement it, where you see many other cases.
Indeed, I tried that approach (it's one of the repos that we include in the README) :-)
There was a problem hiding this comment.
thans for your feedback, appreciated
| for i in range(1, limit + 1): | ||
| if not i % 3 and not i % 5: | ||
| res.append("FizzBuzz") | ||
| elif not i % 3: |
There was a problem hiding this comment.
There is some duplication (e.g. i % 3)
There was a problem hiding this comment.
you are right, should put that in a method of its own
| pass | ||
| def fizzbuzz(limit=100): | ||
| res = [] | ||
| for i in range(1, limit + 1): |
There was a problem hiding this comment.
It might be good to split responsibilities into different components/functions/whatever, so that the responsibility to solve the 100 numbers would be different to the responsibility to decide which answer to be given.
There was a problem hiding this comment.
good point, this was a quick implementation for the workshop, but now with the tests in place we can 'safely' refactor to make it more maintainable
we had fun :)
thanks
we tried list and yield, latter being much faster