pimoroni / unicorn-hat

Python library for Unicorn pHAT and HAT. 32 or 64 blinding ws2812 pixels for your Raspberry Pi
https://shop.pimoroni.com/products/unicorn-hat
MIT License
370 stars 131 forks source link

Ambiguity with snow example #79

Closed r41d closed 8 years ago

r41d commented 8 years ago

In the snow example line 36 says

row[randint(0, width - 1)] = 50 + randint(0, 155)

which yields a value between 50 and 205.

The comment in line 46 says

# val is between 50 and 255

but the maximum value that could come from the above calculation is 205.

Suggestion:

Gadgetoid commented 8 years ago

Good spot, and a good case against comments like this which don't really add any value to what they're commenting. I'd go with option three; delete the comment. Or maybe swap it for something a bit more useful ;D

r41d commented 8 years ago

Well, now it says at the top

MIN_BRIGHTNESS = 50
MAX_BRIGHTNESS = 155

but the maximal possible brightness value from

MIN_BRIGHTNESS + randint(0, MAX_BRIGHTNESS)

is 205, but the reader of the code would think that it should be 155 (line 8).

Unfortunately, it's worse now.

Gadgetoid commented 8 years ago

Well that'll teach me to make drive-by changes over the weekend :D

You're right, MAX_BRIGHTNESS should be 205 and the calculation should be:

MIN_BRIGHTNESS + randint(0, MAX_BRIGHTNESS - MIN_BRIGHTNESS)