Что это за сумасшедшая последовательность циклов и если? Идеи по улучшению этого кода?

Question

Этот код в настоящее время работает, но выглядит ужасно - и, вероятно, может быть значительно улучшен с точки зрения производительности.

Есть предложения?

def OnClick():
    global Selection, touch, set_elsb, set_vreg, set_els, BAR_Items
    A = viz.pick(0, viz.WORLD, all=False)
    if touch != A: return
    for i in BAR_Items:
        if not set_els: break
        elif BAR_Items[i] == A or SHAPES[i+"_SHP"] == A:
            if i in Selection:
                Selection.remove(i)
                BAR_Items[i].clearActions()
                VFrame.SetStatusText(frame, i + " has been deselected")
                viz.director( do_chart )
            else:
                Selection.append(i)

Большое спасибо!

Answer 1

Это еще несколько строк кода, но я думаю, что это понятнее.

def OnClick():
     if not set_els: return

     # swap this with the line above if viz.pick has side effects that should occur 
     A = viz.pick(0, viz.WORLD, all=False) 
     if touch != A: return


     keys = (key for key in BAR_Items
             if BAR_Items[key] == A or SHAPES[key+"_SHP"] == A)

     for key in keys:
         if key in Selection:
             Selection.remove(key)
             BAR_Items[key].clearActions()
             VFrame.SetStatusText(frame, key + " has been deselected")
             viz.director(do_chart)
         else:
             Selection.append(key)

Весь этот оператор global не имел смысла, поскольку вы не назначали ни одному из них. Для вызова атрибутов и ключей настройки не требуется ключевое слово global.

Answer 2

Мой обычный подход к этому состоит в том, чтобы преобразовать некоторые из них в маленькие методы.Это обычно делает его более тестируемым и более легким для чтения.

Answer 3

Очень распространено предположение, что если код уродлив, запутан или труден для понимания, он должен быть неэффективным.

Многие люди также думают, что если вы хотите, чтобы код работал быстрее, вы должны его укрупнить.

Я видел ужасно запутанный код, некоторые из которых работали очень быстро, а другие имели серьезные проблемы с производительностью.

Я также видел чистый, ясный, красивый код, о котором можно сказать то же самое.

Мой опыт - скорость и красота независимы.

Answer 4

Если set_els не изменяется снаружи во время выполнения этого кода, тогда:

def OnClick():
    global Selection, touch, set_elsb, set_vreg, set_els, BAR_Items
    if set_els: return
    A = viz.pick(0, viz.WORLD, all=False)
    if touch != A: return
    for i in BAR_Items:
        if not (BAR_Items[i] == A or SHAPES[i+"_SHP"] == A): continue
        if i in Selection:
            Selection.remove(i)
            BAR_Items[i].clearActions()
            VFrame.SetStatusText(frame, i + " has been deselected")
            viz.director( do_chart )
        else:
            Selection.append(i)

В любом случае, мой детектор плохого кода мигает красным, когда видит такой код, особенно с таким количеством глобальных переменных.