Una funzione può essere troppo breve?

125

Ogni volta che mi ritrovo a scrivere la stessa logica più di una volta, di solito la applico in una funzione, quindi c'è solo un posto nella mia applicazione che devo mantenere quella logica. Un effetto collaterale è che a volte mi ritrovo con una o due funzioni di linea come:

function conditionMet(){
   return x == condition;
}

o

function runCallback(callback){
   if($.isFunction(callback))
     callback();
}

È una pratica pigra o cattiva? Chiedo solo perché questo comporta un numero maggiore di chiamate di funzione per parti di logica molto piccole.

    
posta Mark Brown 01.04.2011 - 21:34
fonte

19 risposte

166

Hehe, oh Mr Brown, se solo riuscissi a persuadere tutti gli sviluppatori che incontro per mantenere le loro funzioni così piccole, credimi, il mondo del software sarebbe un posto migliore!

1) La leggibilità del codice aumenta di dieci volte.

2) È così facile capire il processo del codice a causa della leggibilità.

3) ASCIUTTO - Non ripetere te stesso - ti stai conformando molto bene!

4) Testabile. Piccole funzioni sono un milione di volte più facili da testare rispetto a quei 200 metodi di linea che vediamo troppo spesso.

Oh e non preoccuparti di "funzione hopping" in termini di prestazioni. I build di "Release" e le ottimizzazioni del compilatore si occupano molto bene di questo per noi, e le prestazioni sono il 99% delle volte in un altro posto nella progettazione dei sistemi.

Questo è pigro? - Molto il contrario!

Questa cattiva pratica è questa? - Assolutamente no. Meglio usare questo metodo per creare metodi rispetto alle palle di catrame o "Oggetti di Dio" che sono così comuni.

Continua a fare del buon lavoro, mio collega artigiano;)

    
risposta data 01.04.2011 - 21:27
fonte
63

Direi che un metodo refactored è troppo breve se:

  • Duplica un'operazione primitiva, per nessun altro scopo che renderlo un metodo:

Esempio:

boolean isNotValue() {
   return !isValue();
}

o ...

  • Il codice viene usato una sola volta e il suo intento è facilmente comprensibile a colpo d'occhio.

Esempio:

void showDialog() {
    Dialog singleUseDialog = new ModalDialog();
    configureDialog(singleUseDialog);
    singleUseDialog.show();
}

void configureDialog(Dialog singleUseDialog) {
    singleUseDialog.setDimensions(400, 300);
}

Questo potrebbe essere un modello valido, ma in questo esempio inserirò semplicemente il metodo configureDialog (), a meno che non intendessi sovrascriverlo o riutilizzare altrove questo codice.

    
risposta data 02.04.2011 - 03:49
fonte
59

Una funzione può essere troppo breve? In generale no.

In effetti, l'unico modo per garantire che:

  1. Hai trovato tutte le classi nel tuo progetto
  2. Le tue funzioni stanno facendo solo una cosa.

È quello di mantenere le tue funzioni il più piccole possibile. Oppure, in altre parole, estrai le funzioni dalle tue funzioni fino a quando non puoi estrarne altre. Lo chiamo "Estrai fino allo sfinimento".

Per spiegarlo: una funzione è un ambito con frammenti di funzionalità che comunicano per variabili. Una classe è anche un ambito con frammenti di funzionalità che comunicano per variabili. Quindi una funzione lunga può sempre essere sostituita da una o più classi con un metodo piccolo.

Inoltre, una funzione che è abbastanza grande da permetterti di estrarne un'altra, sta facendo più di una cosa per definizione . Quindi se puoi estrarre una funzione da un'altra, devi dovrebbe estrarre quella funzione.

Alcune persone temono che ciò comporterà una proliferazione di funzioni. Hanno ragione. Lo farà. Questa è davvero una buona cosa. Va bene perché le funzioni hanno nomi. Se fai attenzione a scegliere i buoni nomi, queste funzioni fungono da post di segnalazione che indirizzano altre persone attraverso il tuo codice. Infatti, le funzioni ben denominate all'interno di classi ben definite all'interno di spazi dei nomi ben definiti sono uno dei modi migliori per assicurarsi che i vostri lettori NON si perdano.

C'è molto altro in questo episodio nell'episodio III di Clean Code su cleancoders.com

    
risposta data 02.04.2011 - 01:25
fonte
51

Wow, la maggior parte di queste risposte non è molto utile.

Nessuna funzione dovrebbe essere scritta la cui identità è la sua definizione . Cioè, se il nome della funzione è semplicemente il blocco di codice della funzione scritto in inglese, non scriverlo come una funzione.

Considera la tua funzione conditionMet e questa altra funzione, addOne (perdonami per il mio JavaScript arrugginito):

function conditionMet() { return x == condition; }

function addOne(x) { return x + 1; }

conditionMet è una definizione concettuale corretta; addOne è una tautologia . conditionMet è buono perché non sai cosa conditionMet comporta semplicemente dicendo "condizione soddisfatta", ma puoi vedere perché addOne è sciocco se lo leggi in inglese:

"For the condition to be met is for x to equal condition" <-- explanatory! meaningful! useful!

"To add one to x is to take x and add one." <-- wtf!

Per amore di tutto ciò che potrebbe essere ancora santo, per favore, non scrivere funzioni tautologiche!

(E per lo stesso motivo, non scrivere commenti per ogni riga di codice !)

    
risposta data 12.04.2017 - 09:31
fonte
14

Direi che se pensi che l'intenzione di un certo codice possa essere migliorata aggiungendo un commento, invece di aggiungere quel commento, estrai il codice nel suo metodo. Non importa quanto sia piccolo il codice.

Ad esempio, se il tuo codice doveva essere simile a:

if x == 1 { ... } // is pixel on?

assomiglia a questo:

if pixelOn() { ... }

con

function pixelOn() { return x == 1; }

O in altre parole, non si tratta della lunghezza del metodo, ma del codice di auto-documentazione.

    
risposta data 07.02.2013 - 21:46
fonte
7

Penso che questo sia esattamente ciò che vuoi fare. In questo momento quella funzione potrebbe essere solo una o due righe, ma nel tempo potrebbe crescere. Inoltre avere più chiamate di funzione ti permette di leggere le chiamate di funzione e capire cosa sta succedendo lì dentro. Questo rende il tuo codice molto DRY (Non ripeti te stesso) che è molto più gestibile.

    
risposta data 01.04.2011 - 22:07
fonte
5

Sono d'accordo con tutti gli altri post che ho visto. Questo è un buon stile.

Il sovraccarico di un metodo così piccolo potrebbe essere nullo in quanto l'ottimizzatore potrebbe ottimizzare la chiamata e incorporare il codice. Un semplice codice come questo consente all'ottimizzatore di fare il suo lavoro migliore.

Il codice dovrebbe essere scritto per chiarezza e semplicità. Cerco di limitare un metodo a uno dei due ruoli: prendere decisioni; o eseguire un lavoro. Questo può generare un metodo di linea. Quanto meglio riesco a farlo, tanto più trovo il mio codice.

Il codice come questo tende ad avere un'elevata coesione e un basso accoppiamento che è una buona pratica di codifica.

EDIT: una nota sui nomi dei metodi. Usa un nome di metodo che indica quale metodo non ha il modo in cui lo fa. Trovo verb_noun (_modifier) è un buon schema di denominazione. Questo fornisce nomi come Find_Customer_ByName piuttosto che Select_Customer_Using_NameIdx. Il secondo caso è incline a diventare errato quando il metodo viene modificato. Nel primo caso, è possibile sostituire l'intera implementazione del database del cliente.

    
risposta data 09.06.2011 - 02:19
fonte
4

Il refactoring di una riga di codice in una funzione sembra eccessivo. Potrebbero esserci casi eccezionali, come le righe di verbo loooooong / comples o le expessions, ma non lo farei a meno che io conosca la funzione cresca in futuro.

e il tuo primo esempio suggerisce l'uso di globals (che possono o meno parlare di altri problemi nel codice), lo rifatterei ulteriormente e renderei queste due variabili come parametri:

function conditionMet(x, condition){
   return x == condition;
}
....
conditionMet(1,(3-2));
conditionMet("abc","abc");

L'esempio di conditionMet potrebbe essere utile se la condizione era lunga e ripetitiva come:

function conditionMet(x, someObject){
   return x == ((someObject.valA + someObject.valB - 15.4) / /*...whole bunch of other stuff...*/);
}
    
risposta data 01.04.2011 - 21:43
fonte
3

Considera questo:

Una semplice funzione di rilevamento delle collisioni:

bool collide(OBJ a, OBJ b)
{
    return(pow(a.x - b.x, 2) + pow(a.y - b.y, 2) <= pow(a.radius + b.radius, 2));
}

Se hai scritto quella "semplice" copertina nel tuo codice per tutto il tempo, potresti eventualmente commettere un errore. Inoltre, sarebbe davvero tortuoso scriverlo ancora e ancora.

    
risposta data 02.04.2011 - 10:35
fonte
2

No, e raramente è un problema. Ora, se qualcuno ritiene che nessuna funzione debba essere più lunga di una riga di codice (se solo fosse così semplice), sarebbe un problema e in un certo senso pigro perché non sta pensando a ciò che è appropriato.

    
risposta data 01.04.2011 - 22:17
fonte
2

Direi che sono troppo brevi, ma questa è la mia opinione soggettiva.

A causa:

  • Non c'è motivo di creare una funzione se è usata solo una o due volte. Saltando a destra, fai schifo. Soprattutto con codice VS and C ++ incredibilmente veloce.
  • Panoramica sulla classe. Quando hai migliaia di piccole funzioni, mi fa arrabbiare. Mi piace quando posso visualizzare le definizioni di classe e vedere rapidamente cosa fa, non come SetXToOne, SetYToVector3, MultiplyNumbers, + 100 setters / getter.
  • Nella maggior parte dei progetti questi aiutanti diventano mortali dopo una o due fasi di refactoring, quindi fai "cerca tutto" -> elimina per eliminare il codice obsoleto, di solito ~ 25% + di esso.

Le funzioni lunghe sono cattive, ma le funzioni che sono più corte di 3 righe e eseguono solo 1 cosa sono ugualmente male IMHO.

Quindi direi solo una piccola funzione se è:

  • 3+ linee di codice
  • Le cose che gli sviluppatori junior potrebbero perdere (non sapere)
  • Convalida extra
  • È usato, o verrà utilizzato almeno 3 volte
  • Semplifica l'interfaccia di uso frequente
  • Non diventerà un peso morto durante il prossimo refactoring
  • Ha un significato speciale, ad esempio, la specializzazione del modello o qualcosa
  • Esegue un lavoro di isolamento - i riferimenti const, i parametri mutabili, il recupero dei membri privati

Scommetto che il prossimo sviluppatore (senior) avrà cose migliori da fare che ricordare tutte le funzioni di SetXToOne. Quindi diventeranno presto un peso morto abbastanza presto.

    
risposta data 09.06.2011 - 15:49
fonte
1

Non mi piace l'esempio no. 1, bcause di ith nome generico.

conditionMet non sembra essere generico, quindi rappresenta una condizione specifica? Mi piace

isAdult () = { 
  age >= 18 
}

Questo andrebbe bene. È una differenza semantica, mentre

isAtLeast18 () { age >= 18; } 

non andrebbe bene per me

Forse è spesso usato e può essere oggetto di modifiche successive:

getPreferredIcecream () { return List ("banana", "choclate", "vanilla", "walnut") }

va bene anche Usandolo più volte, devi solo cambiare un singolo posto, se necessario, forse la panna montata diventa possibile domani.

isXYZ (Foo foo) { foo.x > 15 && foo.y < foo.x * 2 }

non è atomico e dovrebbe darti una bella opportunità di test.

Se hai bisogno di passare una funzione, naturalmente, passa tutto quello che vuoi e scrivi funzioni altrimenti stupide.

Ma in generale, vedo molte più funzioni, che sono troppo lunghe, rispetto a funzioni troppo brevi.

Un'ultima parola: alcune funzioni sembrano appropriate, perché sono scritte troppo in profondità:

function lessThan (a, b) {
  if (a < b) return true else return false; 
}

Se vedi, è lo stesso di

return (a < b); 

non avrai problemi con

localLessThan = (a < b); 

invece di

localLessThan = lessThan (a, b); 
    
risposta data 09.06.2011 - 03:51
fonte
0

Non c'è codice come nessun codice!

Semplifica e non complicare le cose.

Non è pigro, sta facendo il tuo lavoro!

    
risposta data 02.04.2011 - 07:06
fonte
0

Sì, è ok per avere una funzione di codice breve. Nel caso di metodi come "getter", "setter", "accessori" è molto comune, come menzionato in precedenza.

A volte, queste brevi funzioni di "accessori" sono virtuali, perché quando vengono sovrascritte in sottoclassi, le funzioni avranno più codice.

Se vuoi che la tua funzione non sia così breve, beh, in molte funzioni, a livello globale o metodi, di solito uso una variabile "risultato" (stile pascal) invece di un ritorno diretto, è molto utile quando si usa un debugger.

function int CalculateSomething() {
  int Result = -1;

   // more code, maybe, maybe not

  return Result;
}
    
risposta data 02.04.2011 - 21:40
fonte
0

Troppo corto non è mai un problema. Alcuni motivi per le funzioni brevi sono:

Riusabilità

es. se hai una funzione, come un metodo set, puoi affermarlo per essere sicuro che i parametri siano validi. Questo controllo deve essere eseguito ovunque sia impostata la variabile.

Maintainability

Potresti utilizzare alcune affermazioni che a tuo avviso potrebbero cambiare in futuro. Per esempio. ora mostri un simbolo in una colonna, ma in seguito potrebbe cambiare in qualcos'altro (o persino un segnale acustico).

uniformità

Stai utilizzando per es. il modello di facciata e l'unica cosa che fa una funzione è esattamente passare la funzione a un'altra senza cambiare argomento.

    
risposta data 14.08.2012 - 16:47
fonte
0

Quando assegni un nome a una sezione di codice, è essenzialmente per dargli un nome . Questo può essere per diversi motivi, ma il punto cruciale è se puoi dargli un nome significativo che aggiunge al tuo programma. Nomi come "addOneToCounter" non aggiungerebbero nulla, ma conditionMet() lo farebbe.

Se hai bisogno di una regola per scoprire se lo snippet è troppo breve o troppo lungo, considera quanto tempo impiega per trovare un nome significativo per lo snippet. Se non puoi farlo entro un ragionevole lasso di tempo, il frammento non è di dimensioni adeguate.

    
risposta data 14.08.2012 - 17:33
fonte
0

No, ma può essere troppo conciso.

Ricorda: il codice viene scritto una volta, ma letto molte volte.

Non scrivere codice per il compilatore. Scrivilo per i futuri sviluppatori che dovranno mantenere il tuo codice.

    
risposta data 08.02.2013 - 01:55
fonte
-1

Sì caro, la funzione potrebbe essere sempre più piccola e se è buona o cattiva dipende dalla lingua / quadro che stai utilizzando.

A mio parere, io lavoro principalmente su Front End Technologies, le Small Functions sono principalmente utilizzate come funzioni di supporto che dovrai utilizzare molto quando lavori con filtri piccoli e usi la stessa logica per tutta l'applicazione. Se la tua applicazione ha troppa logica comune, ci sarà una tonnellata di piccole funzioni.

Ma in un'applicazione in cui non hai una logica comune non sarai obbligato a fare piccole funzioni ma puoi spezzare il tuo codice in segmenti dove diventa facile da gestire e capire.

In generale, rompere il tuo enorme codice in una piccola funzione è un ottimo approccio. Nei moderni framework e linguaggi che dovrai eseguire, ad esempio

data => initScroll(data)

è una funzione anonima in ES 2017 JavaScript e Typescript

getMarketSegments() {
 this.marketService.getAllSegments(this.project.id)
  .subscribe(data => this.segments = data, error => console.log(error.toString()));
}

nel codice sopra puoi vedere 3 Function declaration e 2 function call questa è una semplice Service Call in Angular 4 con Typescript. Puoi pensare ad esso come alle tue esigenze

([] 0)
([x] 1)
([x y] 2)

Quanto sopra sono 3 funzioni anonime nella lingua clojure

(def hello (fn [] "Hello world"))

Il precedente è una dichiarazione funzionale in clojure

Quindi sì FUNCTIONS può essere più piccolo ma se è buono o cattivo se hai funzioni come:

incrementNumber(numb) { return ++numb; }

Beh, non è una buona pratica farlo, ma cosa succede se stai usando questa funzione in un tag HTML come facciamo in Angular Framework se non ci fosse il supporto per Incremento o Decremento nei Modelli HTML angolari, allora sarebbe stato la soluzione per me.

Prendiamo un altro esempio

insertInArray(array, newKey) {
 if (!array.includes(newKey)) {
   array.push(newKey);
 }
}

L'esempio sopra è un must quando si gioca quando array all'interno di modelli HTML angolari. Quindi a volte dovrai creare piccole funzioni

    
risposta data 17.07.2017 - 10:54
fonte
-2

Non sono d'accordo con quasi la risposta data in questo momento.

Recentemente ho trovato un collega che scrive tutti i membri delle classi come il seguente:

 void setProperty(int value){ mValue=value; }
 int getProperty() const { return (mValue); }

Questo potrebbe essere un buon codice in casi sporadici, ma sicuramente non se si definiscono molte classi con molte e molte proprietà. Non voglio dire, con questo, che metodi come quelli sopra non devono essere scritti. Ma se si finisce per scrivere la maggior parte del codice come "wrapper" della logica reale, c'è qualcosa di sbagliato.

Probabilmente il programmatore manca la logica generale, ha paura dei cambiamenti futuri e del refactoring del codice.

La definizione dell'interfaccia è perché un bisogno reale; in effetti se è necessario impostare e ottenere una proprietà semplice (senza molta logica, che rende il membro più breve), sarà possibile. Ma, se il reale bisogno potesse essere analizzato in un modo diverso, perché non definire un'interfaccia più intuitiva?

Per rispondere veramente alla domanda, ovviamente è no, e la ragione è molto ovvia: il metodo / proprietà / whatelse deve essere definito per ciò di cui ha bisogno. Anche una funzione virtuale vuota ha una ragione per esistere.

    
risposta data 02.04.2011 - 00:15
fonte

Leggi altre domande sui tag