2014-01-17 4 views
0

Итак, у меня есть этот модуль под названием risar. И что он делает, он рисует. Но это не имеет особого значения. Я написал этот код, который устанавливает 20 цветов на заднем плане. Код работает, но мне это выглядит ужасно неудобно. Я бы хотел, чтобы он выглядел более «причудливым» или, может быть, было бы использовано меньше циклов. Я относительно новичок в python.То же, но без петель

import risar 
import random 


def makeFlowers(): 
    flowers = [] 
    for i in range(5): 
     colors = ["black_flower.svg","blue_flower.svg", "brown_flower.svg", "green_flower.svg","purple_flower.svg"] 
     for j in range(4): 
      x = random.randint(20, (risar.maxX-20)) 
      y = random.randint(20, 300) 
      flower = risar.picture(x, y, colors[i]) 
      flowers.append(flower) 
    return flowers 

flowers = makeFlowers() 
+0

Фактор из 'colors', так как он остается постоянным. – arshajii

+5

Этот вопрос не соответствует теме, потому что речь идет о просмотре кода. Должен быть на сайте проверки стека кода (http://codereview.stackexchange.com/). –

+0

Я не знал, что существует. Спасибо, что сообщили мне, оцените его и обязательно его исповедуют! xD – Doe

ответ

2

Ну для начала, вы устанавливаете color переменную в этом же списке, в пять раз, так что вы можете удалить, что из цикла:

# Just set `colors` once 
colors = ["black_flower.svg","blue_flower.svg", "brown_flower.svg","green_flower.svg","purple_flower.svg"] 
for i in range(5): 
    # Do stuff 

Во-вторых, вы используете две петли без всякой причины. Подсказка: если вы не используете j для чего-либо, он может не заслужить своего собственного цикла.

Вместо этого вытащите код, который вы хотите повторить, а затем повторите его много раз. В этом случае вы хотите повторить блок кода двадцать раз, но вы используете вложенные циклы, чтобы убедиться, что вы вращаетесь по цветам. Вместо этого, обратите внимание, что количество цветов вы рисуете делится на длину вашего списка цветов, и реорганизовать все это в одном цикле:

for i in range(20): 
    x = random.randint(20, (risar.maxX-20)) 
    y = random.randint(20, 300) 
    flower = risar.picture(x, y, colors[i//4]) # 0, 0, 0, 0, 1, 1, 1, 1, etc... 
    flowers.append(flower) 
+0

Гораздо лучше, я бы просто заменил диапазон для функции xrange – rekiem87

+0

@ rekiem87 Прибыль от 'xrange' vs' range' в 20- петля элементов чрезвычайно минимальны. Кроме того, OP не указал версию Python, а 'range' в Python3 работает как' xrange' в Python 2 (и 'xrange' больше не существует!) –

1

Что-то вроде

colors = ["black_flower.svg","blue_flower.svg", "brown_flower.svg", "green_flower.svg","purple_flower.svg"] 
for color in colors: 
    //You can do your second for here, no need to redeclare colors in each for, and color will be taking each value of the array 

И только еще одна вещь, вам лучше использовать

xrange(limit) 

разница заключается в том, что xrange является более эффективным, так как не создает список чисел, только генератор последовательности, это очень важно в тяжелых мем ory, как картинка

Смежные вопросы