Lasciando commenti "Else"

2

Diciamo che ho un'istruzione non-così-intuitiva if nel mio codice, ma solo se sei nuovo alla base di codice:

def set_markets(markets=None):
    """
    Will accept 'all' as markets to set all markets on.
    For individual markets, pass in a list of ints.
    """
    if markets:
        if type(markets) == list:
            markets = ','.join(map(str, markets))
        self._set_markets(markets)

Per quelli di voi che si chiedono:

Il sistema esistente con cui sto interagendo (un database) si aspetta un elenco di numeri interi dall'interfaccia front-end. Questo codice lo fa dal back-end, quindi è stata scelta una lista di ints poiché rappresenta al meglio ciò che un utente potrebbe nutrire altrimenti.

La roba markets viene tramandata da un'altra classe, quindi anche se sembra che si svolga su 5 righe, è più simile a ~ 50 linee.

La mia domanda è, lo considereresti più leggibile? Perché o perché no?

def set_markets(markets=None):
    """
    Will accept 'all' as markets to set all markets on.
    For individual markets, pass in a list of ints.
    """
    if markets:
        if type(markets) == list:
            markets = ','.join(map(str, markets))
      # else:
          # markets are set to "all"
        self._set_markets(markets)

EDIT:

Questo codice sembra peggiore di quello che è perché ho omesso un dettaglio: se None è alimentato per markets (il parametro predefinito), quindi non modificherà le impostazioni dei mercati del database.

Ecco cosa ho ora:

    if markets is not None:
        if isinstance(markets, list):
            markets = ','.join(map(str, markets))
        else:
            markets = 'all'
        self._set_new_header_markets(markets)
    
posta Droogans 04.03.2013 - 21:46
fonte

2 risposte

13

In primo luogo, inserire il codice nei commenti mi fa pensare che sia un codice morto. Sarà davvero molto confuso se lo fai.

In secondo luogo, il fatto che tu abbia bisogno di un commento è perché il tuo codice è illeggibile. Cerca di rendere leggibile il codice prima di aggiungere un commento.

Se accetti "tutto" o un elenco, dovresti farlo in questo modo:

if markets == 'all':
   self._set_markets('all')
else:
   self._set_markets(','.join(map(str, markers))

Quindi è abbondantemente chiaro su cosa sta succedendo e si evita il controllo dei tipi, e non si accettano dati non validi finché non si tratta di un elenco.

    
risposta data 04.03.2013 - 22:10
fonte
2

Per me, il fatto che tu abbia formattato else: come un normale codice è fuorviante; Qualche commento in più sembra appropriato.

In questo momento sembra che l'altro sia commentato perché non esiste un codice per quel blocco ancora , e il commento interno è un segnaposto per il codice che verrà, che non è il caso (?) . Inserirò entrambi i commenti, inserirò il rientro sull'hash e forse rimuovere la formattazione dell'altro:

if markets:
    if type(markets) == list:
        markets = ','.join(map(str, markets))
    # else: markets are set to "all"
    self._set_markets(markets)
    
risposta data 04.03.2013 - 22:12
fonte

Leggi altre domande sui tag