# How clean is my snow?

Posted on

Problem

I just wrote a snow animation in Python. I think this is fairly clean but I have a few things that I don’t like.

``````from random import randrange
import time

# Snow animation
# Snow is simply the `#` symbol here. Half of the snow moves at 1 char
# per frame, while the other half moves at 0.5 chars per frame. The
# program ends when all the snow reaches the bottom of the screen.
# The viewing area is 80x25. It puts 100 snow flakes to output, half
# fast, and half slow. Every frame it dispenses 4 flakes, 2 fast and
# 2 slow ones, at random locations at the top of the viewing area.

screen = {'x': 80, 'y': 20}
drops = []

def createRainDrop(x, y, speed):
return {'x': x, 'y': y, 'speed': speed}

def createRandomDrops():
dropCount = 4
for i in range(dropCount):
yield createRainDrop(randrange(0, screen['x']), 0, min((i % 2) + 0.5, 1))

def moveDrops():
for drop in drops:
speed = drop['speed']
drop['y'] = drop['y']+speed

def drawDrops():
out = [''] * screen['y']
for y in range(screen['y']):
for x in range(screen['x']):
out[y] += '#' if any([drop['x'] == x and int(drop['y']) == y for drop in drops]) else ' '

return 'n'.join(out)

def dropsOnScreen():
return any([drop['y'] < screen['y'] for drop in drops])

drops += createRandomDrops()

while dropsOnScreen():
if len(drops) < 100:
drops += createRandomDrops()

print(drawDrops())
moveDrops()
time.sleep(0.100)
``````

For example, I don’t know how to remove the duplicate line `drops += createRandomDrops()`, and `drawDrops()` feels a bit like a hack.

I confess! While writing this it was rain, not snow!

Solution

Let’s look at the code.

``````from random import randrange
import time
``````

Your imports are very minimal! Good.

``````# Snow animation
# Snow is simply the `#` symbol here. Half of the snow moves at 1 char
# per frame, while the other half moves at 0.5 chars per frame. The
# program ends when all the snow reaches the bottom of the screen.
# The viewing area is 80x25. It puts 100 snow flakes to output, half
# fast, and half slow. Every frame it dispenses 4 flakes, 2 fast and
# 2 slow ones, at random locations at the top of the viewing area.
``````

This looks more like a docstring to me. It would be nice to render it as such. You can do this by dropping the `#` signs, and surrounding it in `"""` quotes.

``````screen = {'x': 80, 'y': 20}
drops = []
``````

Global variables are not that nice. But this is a simple file, so maybe we can leave it like this for now? Let’s.

``````def createRainDrop(x, y, speed):
return {'x': x, 'y': y, 'speed': speed}
``````

I think something like a class would be better for this. Let’s try

``````class RainDrop(object):
def __init__(self, x, y, speed):
self.x = x
self.y = y
self.speed = speed
``````

Of course, now we need to replace `createRainDrop(...)` with `RainDrop(...)`, and `drop['...']` with `drop....`.

``````def createRandomDrops():
dropCount = 4
for i in range(dropCount):
yield RainDrop(randrange(0, screen['x']), 0, min((i % 2) + 0.5, 1))
``````

That’s better.

``````def moveDrops():
for drop in drops:
drop.y = drop.y + drop.speed
``````

We’re modifying `drop` here, instead of asking it to modify itself. We should be writing something like `drop.moveDown()` here, or maybe `drop.tick()` (‘tick’ is what’s commonly used to notify an event about stepping forward in time).

``````def drawDrops():
out = [''] * screen['y']
for y in range(screen['y']):
for x in range(screen['x']):
out[y] += '#' if any([drop.x == x and drop.y == y for drop in drops]) else ' '

return 'n'.join(out)
``````

Here, for all the positions on the screen, you’re looping over all the drops. Ideally we’d turn that around:

``````def drawDrops():
out = [[' ' for _ in range(screen['x'])] for _ in range(screen['y'])]
for drop in drops:
if int(drop.y) < screen['y']:
out[int(drop.y)][drop.x] = '#'
``````

Now that’s a bit faster and cleaner.

``````def dropsOnScreen():
return any([drop.y < screen['y'] for drop in drops])
``````

Makes sense. Except I’d suggest to not use the `[...]`, which creates a list. Better to use

``````def dropsOnScreen():
return any(drop.y < screen['y'] for drop in drops)
``````

This behaves the same, but does not have to create an intermediate list.

``````drops += createRandomDrops()

while dropsOnScreen():
if len(drops) < 100:
drops += createRandomDrops()

print(drawDrops())
moveDrops()
time.sleep(0.100)
``````

You want to get rid of the duplicated call to `drops += createRandomDrops()`.

``````while True:
if len(drops) < 100:
drops += createRandomDrops()

if not dropsOnScreen():
break

print(drawDrops())
moveDrops()
time.sleep(0.100)
``````

But in my opinion, the extra `createRandomDrops` is not that bad.

Cool animation!

Let’s get some linting out of the way. As per PEP 8, you should use 4 spaces of indentation consistently, and function names should be `snake_case`.

## Scalability

The main weakness of your design is scalability. If you extend the loop to run indefinitely, then you will eventually run into performance issues.

One problem is that the `drops` list grows with each iteration, and is never pruned. The drops don’t disappear after falling to the ground; they keep falling forever, invisibly, off-screen. The solution is to have `moveDrops()` delete drops when they fall beyond the bottom. (That’s a smarter strategy than having `dropsOnScreen()` re-examine every drop on every animation frame.)

Furthermore, to place the drops on the grid, you do an O(n) scan for each position on the screen with `'#' if any([drop['x'] == x and int(drop['y']) == y for drop in drops])`. I would rewrite `drawDrops()` so that each drop places itself, using a dictionary or a 2-D array. I would also prefer to use comprehensions than repeated append operations, but that is mostly a style preference.

## Data types

Your comment says that the screen dimensions are 80×25, but your code says `screen = {'x': 80, 'y': 20}`. Ideally, the dimensions should be detected at runtime using the `curses` library. Since `screen` is used as a global variable, I would like to see it named `SCREEN` and made immutable. A `namedtuple` would make it immutable, with the additional benefit of allowing the dot accessor rather than the clumsy `[]` notation. I think that `width` and `height` would be more appropriate names than `x` and `y`.

Similarly, defining a class for the raindrops would avoid the `drop['x']` notation. Furthermore, the `createRainDrop()` function cries out to be a constructor.

## Creating drops and looping

The rest of the code is an exercise in Pythonic iteration. Everything can be handled with liberal usage of iterators.

In `createRandomDrops()`, instead of the cryptic formula `min((i % 2) + 0.5, 1)`, use `itertools.cycle([0.5, 1])`. I would turn `createRandomDrops()` into an infinite generator.

In the solution below, parameters such as the speed, intensity, and duration are all centrally tweakable by modifying `drop_params` and `precipitation`. For example, `precipitation = drop_generator(**drop_params)` would result in an infinite loop with just one new drop per frame.

## Suggested solution

``````from collections import namedtuple
import curses
from itertools import chain, cycle, islice, repeat
from random import randrange
import sys
import time

SCREEN = namedtuple('Screen', 'height width')(*curses.initscr().getmaxyx())
curses.endwin()

class Raindrop:
def __init__(self, x, y, speed):
self.x, self.y, self.speed = x, y, speed

def drop_generator(batch_size=1, **drop_params):
while True:
yield [
Raindrop(**{key: next(gen) for key, gen in drop_params.items()})
for _ in range(batch_size)
]

def move_drops(drops):
"""Move each drop down according to its speed, and remove drops from the
set that have fallen off."""
for drop in drops:
drop.y += drop.speed
drops.difference_update([drop for drop in drops if drop.y >= SCREEN.height])

def render_drops(drops, char='#'):
"""Return a string representation of the entire screen."""
scr = {
int(drop.y) * SCREEN.width + int(drop.x): char for drop in drops
}
return 'n'.join(
''.join(scr.get(y * SCREEN.width + x, ' ') for x in range(SCREEN.width))
for y in range(SCREEN.height)
)

drop_params = {
'x': (randrange(0, SCREEN.width) for _ in repeat(True)),
'y': repeat(0),
'speed': cycle([0.5, 1]),
}
precipitation = chain.from_iterable([
islice(drop_generator(batch_size=4, **drop_params), 25),
repeat([])  # ... then generate nothing as existing drops keep falling
])
drops = set(next(precipitation))
while drops:
drops.update(next(precipitation))
print(render_drops(drops))
# Python 2.7 seems to have a curses bug that necessitates flushing
sys.stdout.flush()
move_drops(drops)
time.sleep(0.100)
``````

You don’t get `drops` of snow! Clearly it should be

``````  for flake in flurry:
``````

I’m surprised no one has talked about your character choice! Why are their a bunch of hashtags falling? Nah, I kid, it was an ok choice of character, but we can do better! What about changing the `#` to the unicode (which Python 3 supports!) `❄`. Now it really looks like snow!

Also, your code at the moment is backwards compatible with Python 2. My change of character would break that. If you want to fix that add:

``````# -*- coding: utf8 -*-
``````

To the top of your file.

``````def createRandomDrops():
dropCount = 4
for i in range(dropCount):
``````

Magic number 4 in middle of the code

``````def drawDrops():
``````

I would expect this method to actually draw drops, not return string to print