« Petite revue de code » — GuieA_7 @ linuxfr.org

[nojhan]

# Petite revue de code

Posté par GuieA_7 (site Web personnel) le 20/08/22 à 13:18. Évalué à 10 (+10/-0).

Bonjour,

je ne vais pas faire une PR GitHub (je ne saurai pas tester que je n'ai rien cassé). Mais comme j'aime bien relire du Python, je me permets quelques petites remarques, parce que le code est plutôt de bonne facture.

À beaucoup d'endroits (pas partout) on trouve des logging.debug("Log in %s" % logfile) ; il est préférable d'écrire logging.debug("Log in %s", logfile) car l'interpolation de chaîne ne sera faite que si le message est affiché (le niveau est DEBUG ou moins dans cet exemple).

logging.debug("ici: %i %s", i, cmd[i]) j'imagine que le "ici" est un reliquat.

if type(tunnel) == AutoTunnel: je pense qu'un if isinstance(tunnel, AutoTunnel): est plus propre (même si au final tester les types c'est pas très élégant, et genre une méthode/propriété "is_auto", par exemple, serait plus appropriée je trouve).

if t.status != 'ESTABLISHED' and t.status != 'LISTEN': => if t.status in ('ESTABLISHED', 'LISTEN'):

Il y a beaucoup d'utilisation de eval() qui gagneraient à être remplacées par des getattr() ; outre le caractère sécurité, on évite aussi de reparser du code à chaque fois (eval() est vraiment à éviter ; je doute m'en être servi en presque 20 ans de Python). Dans la mesure où Python 3.8 est requis, tu peux utiliser à moult endroits les f-strings qui sont vraiment super sympathiques.

Il y a beaucoup de double-recherches dans les dictionnaires, quand une recherche simple sera plus efficace et souvent plus compacte.

if forward in self.forwards:
    self.forward = self.forwards[forward]
else:
    self.forward = "unknown"

peut s'écrire simplement self.forward = self.forwards.get(forward, "unknown").

De la même manière le bout de code suivant est intéressant (car plusieurs améliorations sont possibles)

    def __repr__(self):
        reps = [self.header]
        for t in self.tunnels:
            reps.append(str(self.tunnels[t]))
        return "\n".join(reps)

On utilise .values() vu qu'on n'a pas besoin de la clé:

    def __repr__(self):
        reps = [self.header]
        for tunnel in self.tunnels.values():
            reps.append(str(tunnel))
        return "\n".join(reps)

On utilise .extend() et une generator expression pour éviter d'avoir N appels à append() tout en étant plus court:

    def __repr__(self):
        reps = [self.header]
        # Remarque 1: pas besoin de mettre une autre paire de parenthèses car le generator est le seul paramètre
        reps.extend(str(tunnel) for tunnel in self.tunnels.values())
        # Remarque 2: version alternative avec map()
        # reps.extend(map(str, self.tunnels.values()))

        return "\n".join(reps)

Avec les nouvelles fonctionnalités d'unpacking de Python 3, on arrive à ce résultat qui a un petit goût de programmation fonctionnelle plutôt cool:

    def __repr__(self):
        return "\n".join([self.header, *map(str, self.tunnels.values())])

if c.raddr:
    raddr, rport = c.raddr
else:
    raddr, rport = (None, None)

==> raddr, rport = c.raddr or (None, None)

En espérant avoir été utile. Bon week-end !