2012-09-03 3 views
0

Cela peut être une question assez verte, mais j'espère que vous comprenez - juste commencé sur Python et en essayant d'améliorer. Quoi qu'il en soit, a écrit une petite fonction pour faire la "méthode de lacet" de trouver la zone d'un polygone dans un plan cartésien (voir this pour un rappel).Conseils pour améliorer cette fonction?

Je veux savoir comment je peux améliorer ma méthode, afin que je puisse essayer de nouvelles façons de faire les mêmes vieilles choses.

def shoelace(list): 
     r_p  = 0   # Positive Values 
     r_n  = 0   # Negative Values 

     x, y = [i[0] for i in list], [i[1] for i in list] 
     x.append(x[0]), y.append(y[0]) 

     print(x, y) 

     for i in range(len(x)): 
      if (i+1) < len(x): 
       r_p += (x[i] * y[i+1]) 
       r_n += (x[i+1] * y[i]) 
      else: 
       break 

     return ((abs(r_p - r_n))/2) 
+2

Vous devriez poster ceci sur la révision du code de site SE. – Usagi

+0

Oh ok merci pour la mise à jour. –

+4

Un conseil: 'list' est un mauvais nom pour une variable car c'est le nom d'un built-in. –

Répondre

0

Fonctionnellement, votre programme est assez bon. Une remarque mineure est de remplacer range(len(x)) par xrange(len(x)). Cela rend le programme légèrement plus efficace. Généralement, vous devez utiliser range uniquement dans les cas où vous avez réellement besoin de la liste complète des valeurs qu'il crée. Si tout ce dont vous avez besoin est de faire une boucle sur ces valeurs, utilisez xrange. En outre, vous n'avez pas besoin de parenthèses dans l'instruction return, ni dans les instructions r_p += et r_n +=.

En ce qui concerne le style, dans les affectations de variables Python ne doivent pas se faire comme vous l'avez fait, mais avec un seul espace de chaque côté du symbole =:

r_p = 0 
r_n = 0 
+0

Merci pour les suggestions :) –

+0

De rien. Assurez-vous de ne pas manquer la suggestion de Burhan, qui m'a manqué (lundi matin): 'list' est un nom terrible pour une variable. – HerrKaputt

2
  • Ne pas utiliser la variable courte les noms qui doivent être commentés; utiliser des noms qui indiquent la fonction. Est le nom du type de liste intégré, alors que Python vous laisse remplacer ce nom, c'est une mauvaise idée stylistique.

  • , ne doit pas être utilisé pour séparer ce qui est supposé être des instructions. Vous pouvez utiliser ;, mais il est généralement préférable de placer les choses sur des lignes distinctes. Dans votre cas, cela fonctionne parce que vous utilisez .append pour l'effet secondaire, mais en gros ce que vous faites est de construire le 2-tuple (None, None) (les valeurs de retour de .append) et de le jeter.

  • Utilisez les fonctions intégrées si possible pour les transformations de liste standard. Voir la documentation pour zip, par exemple. Sauf que vous n'avez pas vraiment besoin d'effectuer cette transformation; vous voulez considérer des paires de points adjacents, alors faites cela - et démontez leurs coordonnées à l'intérieur de la boucle.

  • Cependant, vous pouvez utiliser zip pour transformer la liste des points dans une liste de paires de points adjacents :) qui vous permet d'écrire une boucle beaucoup plus propre. L'idée est simple: d'abord, nous faisons une liste de tous les "prochains" points par rapport aux originaux, et ensuite nous avons zip ensemble les deux points-listes.

  • return La fonction return ne nécessite pas de parenthèses.

  • Au lieu de compter des valeurs positives et négatives séparées, effectuez une arithmétique signée sur une seule valeur.


def shoelace(points): 
    signed_double_area = 0 

    next_points = points[1:] + points[:1] 

    for begin, end in zip(points, next_points): 
     begin_x, begin_y = begin 
     end_x, end_y = end 
     signed_double_area += begin_x * end_y 
     signed_double_area -= end_x * begin_y 

    return abs(signed_double_area)/2 
Questions connexes