Funzioni a una riga chiamate solo una volta

116

Considera una funzione senza parametri ( modifica: non necessariamente) che esegue una singola riga di codice e viene chiamata una sola volta nel programma (anche se non è impossibile che sarà nuovamente necessaria in il futuro).

Potrebbe eseguire una query, controllare alcuni valori, fare qualcosa che riguarda la regex ... qualsiasi cosa oscura o "hacky".

La logica alla base di questo sarebbe evitare valutazioni difficilmente leggibili:

if (getCondition()) {
    // do stuff
}

dove getCondition() è la funzione a linea singola.

La mia domanda è semplicemente: è una buona pratica? Mi sembra giusto ma non so a lungo termine ...

    
posta vemv 12.09.2011 - 16:34
fonte

11 risposte

237

Dipende da quella linea. Se la linea è leggibile e concisa da sola, la funzione potrebbe non essere necessaria. Esempio semplicistico:

void printNewLine() {
  System.out.println();
}

OTOH, se la funzione dà un buon nome a una riga di codice contenente per es. un'espressione complessa, difficile da leggere, è perfettamente giustificata (per me). Esempio contrato (suddiviso in più righe per la leggibilità qui):

boolean isTaxPayerEligibleForTaxRefund() {
  return taxPayer.isFemale() 
        && (taxPayer.getNumberOfChildren() > 2 
        || (taxPayer.getAge() > 50 && taxPayer.getEmployer().isNonProfit()));
}
    
risposta data 12.09.2011 - 16:36
fonte
66

Sì, questo può essere usato per soddisfare le migliori pratiche. Ad esempio, è meglio avere una funzione con nomi chiari che faccia funzionare, anche se è solo una riga, che avere quella linea di codice all'interno di una funzione più grande e che necessiti di un commento a una riga che spieghi cosa fa. Inoltre, le linee di codice vicine devono eseguire attività allo stesso livello di astrazione. Un controesempio sarebbe qualcosa come

startIgnition();
petrolFlag |= 0x006A;
engageChoke();

In questo caso è decisamente meglio spostare la linea di mezzo in una funzione con nome sensato.

    
risposta data 12.09.2011 - 16:39
fonte
37

Penso che in molti casi tale funzione abbia un buon stile, ma potresti considerare una variabile booleana locale come alternativa nei casi in cui non devi usare questa condizione da qualche altra parte, ad esempio:

bool someConditionSatisfied = [complex expression];

Questo darà un suggerimento al lettore di codice e consentirà di risparmiare dall'introduzione di una nuova funzione.

    
risposta data 12.09.2011 - 17:15
fonte
23

Oltre a risposta di Peter , se questa condizione potrebbe dover essere aggiornata in futuro, facendola incapsulare come suggerisci di avere un solo punto di modifica.

Seguendo l'esempio di Peter, se questo

boolean isTaxPayerEligibleForTaxRefund() {
  return taxPayer.isFemale() 
        && (taxPayer.getNumberOfChildren() > 2 
        || (taxPayer.getAge() > 50 && taxPayer.getEmployer().isNonProfit()));
}

diventa questo

boolean isTaxPayerEligibleForTaxRefund() {
  return taxPayer.isMutant() 
        && (taxPayer.getNumberOfThumbs() > 2 
        || (taxPayer.getAge() > 123 && taxPayer.getEmployer().isXMan()));
}

Esegui una singola modifica e viene aggiornata universalmente. Mantenibilità, questo è un vantaggio.

Per quanto riguarda le prestazioni, i compilatori più ottimizzanti rimuoveranno comunque la chiamata alla funzione e incorporeranno il blocco di codice ridotto. Ottimizzare qualcosa del genere può effettivamente ridurre le dimensioni del blocco (eliminando le istruzioni necessarie per la chiamata di funzione, il ritorno, ecc.) Quindi è normalmente sicuro anche in condizioni che potrebbero altrimenti impedire l'inlining.

    
risposta data 12.09.2011 - 17:13
fonte
5

Oltre alla leggibilità (o in complemento di esso) ciò consente di scrivere funzioni al giusto livello di astrazione.

    
risposta data 12.09.2011 - 20:00
fonte
4

Dipende. A volte è meglio incapsulare un'espressione in una funzione / metodo, anche se è solo una riga. Se è complicato da leggere o ne hai bisogno in più punti, allora lo considero una buona pratica. A lungo termine è più facile da mantenere, poiché hai introdotto un singolo punto di cambiamento e una migliore leggibilità .

Tuttavia, a volte è solo qualcosa che non ti serve. Quando l'espressione è facile da leggere comunque e / o appare in un unico punto, non avvolgerlo.

    
risposta data 12.09.2011 - 16:38
fonte
3

Penso che se ne hai solo alcuni, allora va bene, ma il problema si presenta quando ce ne sono molti nel tuo codice. E quando il compilatore viene eseguito o l'interpitor (a seconda della lingua che si usa) sta andando a quella funzione in memoria. Quindi, diciamo che ne hai 3, non penso che il computer noterà, ma se inizi ad avere 100 di quelle piccole cose, il sistema deve registrare le funzioni in memoria che vengono chiamate solo una volta e quindi non distrutte.

    
risposta data 13.09.2011 - 07:36
fonte
3

Ho fatto esattamente questa cosa di recente in un'applicazione che ho effettuato il refactoring, per rendere esplicito il significato reale dei commenti del codice sans:

protected void PaymentButton_Click(object sender, EventArgs e)
    Func<bool> HaveError = () => lblCreditCardError.Text == string.Empty && lblDisclaimer.Text == string.Empty;

    CheckInputs();

    if(HaveError())
        return;

    ...
}
    
risposta data 12.09.2011 - 20:33
fonte
0

Spostare quella linea in un metodo con un buon nome facilita la lettura del codice. Molti altri hanno già detto che ("codice di auto-documentazione"). L'altro vantaggio di spostarlo in un metodo è che rende più facile il test dell'unità. Quando è isolato nel suo stesso metodo e l'unità è testata, puoi essere certo che se / quando viene trovato un bug, non sarà in questo metodo.

    
risposta data 16.09.2011 - 06:15
fonte
0

Ci sono già molte buone risposte, ma c'è un caso speciale che vale la pena menzionare .

Se la tua istruzione su una sola riga richiede un commento e sei in grado di identificare chiaramente (il nome: nome) il suo scopo, considera l'estrazione di una funzione migliorando il commento nel documento API. In questo modo, la chiamata alla funzione diventa più facile e comprensibile.

È interessante notare che lo stesso può essere fatto se al momento non c'è nulla da fare, ma un commento che ricordi le espansioni necessarie (nel prossimo futuro 1) ), quindi questo,

def sophisticatedHello():
    # todo set up
    say("hello")
    # todo tear down

potrebbe anche essere cambiato in questo

def sophisticatedHello():
    setUp()
    say("hello")
    tearDown()

1) dovresti essere veramente sicuro di questo (vedi il principio YAGNI )

    
risposta data 09.01.2015 - 17:08
fonte
0

Se il linguaggio lo supporta, di solito uso le funzioni anonime etichettate per realizzare questo.

someCondition = lambda p: True if [complicated expression involving p] else False
#I explicitly write the function with a ternary to make it clear this is a a predicate
if (someCondition(p)):
    #do stuff...

IMHO questo è un buon compromesso perché ti dà il vantaggio di leggibilità di non avere la complicata espressione che ingombrante la condizione if evitando di ingombrare lo spazio dei nomi globale / pacchetto con etichette di piccole dimensioni. Ha il vantaggio aggiunto che la funzione "definizione" è giusta dove viene utilizzata, facilitando la modifica e la lettura della definizione.

Non deve essere solo funzioni di predicato. Mi piace racchiudere la piastra della caldaia ripetuta in piccole funzioni come questa (Funziona particolarmente bene per la generazione di elenchi pitonici senza ingombrare la sintassi della parentesi). Ad esempio, il seguente esempio semplificato quando si lavora con PIL in python

#goal - I have a list of PIL Image objects and I want them all as grayscale (uint8) numpy arrays
im_2_arr = lambda im: array(im.convert('L')) 
arr_list = [im_2_arr(image) for image in image_list]
    
risposta data 13.09.2011 - 02:43
fonte

Leggi altre domande sui tag