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

Drawing and moving flowers

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

Problem

So I've got this module called risar, which draws. But that's not really of importance. I wrote this code which sets 20 flowers on the background.

Now they need to move downwards by 5 units. The code works fine but it looks horribly awkward to me. I'd like it to look more "fancy" or maybe that less loops would be used or I'd like to see simply another way to do it. I need help with 2nd function; the 1st one looks okay. I'm relatively new to Python.

def doFlowers(): 
    flowers = []
    colors = ["black_flower.svg","blue_flower.svg", "brown_flower.svg", "green_flower.svg","purple_flower.svg"]
    for i in range(20):
        x = random.randint(20, (risar.maxX-20))
        y = random.randint(20, 300)
        flower = risar.picture(x, y, barve[i//4]) # 0, 1, 2, 3, 4
        flowers.append(flower)
    return flowers
flowers = doFlower()

def moving(flower):
    for i in range(len(flower)):
        while flower[i].y() < risar.maxY:
            for p in flower: #p is position
                p.setPos(p.x(), p.y()+5)

Solution

Let's start with your naming. flower is supposed to be a list, right? Let's make it plural, then.

def moving(flowers):
    for i in range(len(flowers)):
        while flowers[i].y() < risar.maxY:
            for p in flowers: #p is position
                p.setPos(p.x(), p.y()+5)


Next, notice that you don't actually care about i; you just want to operate on every single flower.

def moving(flowers):
    for f in flowers:
        while f.y() < risar.maxY:
            for p in flowers: #p is position
                p.setPos(p.x(), p.y()+5)


Hmm… not only are there nested loops, but two of them iterate over all flowers. What's going on? I think you're trying to move all flowers "simultaneously" until all of them have their y-coordinate ≥ risar.maxY. Then just say this, which I think is a bit more expressive:

def moving(flowers):
    while any([f.y() < risar.maxY for f in flowers]):
        for flower in flowers:
            flower.setPos(flower.x(), flower.y() + 5)

Code Snippets

def moving(flowers):
    for i in range(len(flowers)):
        while flowers[i].y() < risar.maxY:
            for p in flowers: #p is position
                p.setPos(p.x(), p.y()+5)
def moving(flowers):
    for f in flowers:
        while f.y() < risar.maxY:
            for p in flowers: #p is position
                p.setPos(p.x(), p.y()+5)
def moving(flowers):
    while any([f.y() < risar.maxY for f in flowers]):
        for flower in flowers:
            flower.setPos(flower.x(), flower.y() + 5)

Context

StackExchange Code Review Q#39506, answer score: 3

Revisions (0)

No revisions yet.