Qu'advient-il de cette folle séquence de boucles et de si? Des idées pour améliorer ce code?

Question

Ce code fonctionne actuellement, mais il a l'air terrible - et peut probablement être beaucoup amélioré en termes de performance.

Avez-vous des suggestions?

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) 

Merci beaucoup!

Answer 1

C'est un peu plus de lignes de code mais je pense que c'est plus clair.

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) 

que toute déclaration global ne sert à rien que vous n'assignant à aucun d'entre eux. Les attributs d'appel et les clés de configuration ne nécessitent pas le mot-clé global.

Answer 2

Mon approche habituelle à cela serait d'en factoriser une partie en petites méthodes. Cela le rend généralement plus testable et plus facile à lire.

Answer 3

Si set_els n'est pas changé à l'extérieur pendant cette exécution de code puis:

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) 

Quoi qu'il en soit, mon mauvais détecteur clignote de code avec la lumière rouge quand il voit un tel code, en particulier avec une telle quantité de globals.

Answer 4

Il est très commun de supposer que si le code est moche, confus ou difficile à suivre, il doit donc être inefficace.

Beaucoup de gens pensent aussi que si vous voulez faire du code aller plus vite, vous devez le gâcher.

J'ai vu un code très confus, dont certains fonctionnaient très rapidement, et d'autres avaient des problèmes de performance massifs.

J'ai également vu propre, clair, beau code dont la même chose pourrait être dit.

Mon expérience - la vitesse et la beauté sont indépendantes.