Si sta dividendo una funzione in più funzioni interne e un anti-pattern? [duplicare]

Le funzioni interne non sono anti-pattern, sono una caratteristica.

Se non ha senso spostare le funzioni interne all'esterno, quindi con tutti i mezzi, non farlo.

D'altra parte, sarebbe una buona idea spostarli all'esterno in modo da poterli testare più facilmente. (Non so se qualche framework ti permette di testare le funzioni interne.) Quando hai una funzione con 250+ linee, e fai qualche cambiamento nella logica, sei sicuro di non rompere nulla? Hai davvero bisogno di test unitari lì, e non sarà fattibile con una funzione gigantesca.

Dividere le funzioni lunghe in quelle più piccole è una buona idea in generale. È una tecnica di refactoring comune per sostituire un commento con una funzione chiamata dopo il commento, chiamata "metodo di estrazione" . Quindi sì, fallo!

Come sottolineato @Kilian , vedi anche questo post correlato:

Devo posizionare le funzioni che sono utilizzate solo in un'altra funzione, all'interno di quella funzione?

Non è un anti-modello. È il modo giusto per farlo.

È più facile da leggere, da mantenere, da testare. Aiuta ad evitare la duplicazione.

Vedi: codice pulito (la bibbia per gli sviluppatori) e questa risposta

Il problema più importante è gestire la complessità. I metodi interni sono una delle soluzioni, ma non sono convinto che sia il migliore.

Vuoi dividere il metodo perché è lungo, ma con i metodi interni il metodo è ancora lungo, diviso in "capitoli". È meglio di "commenti capitoli", però.

Una soluzione migliore consiste nell'utilizzare una classe interna che raccoglie tutti i metodi di supporto e - ciò che è anche importante - i dati. Inoltre, questa classe può essere sottoposta a test dell'unità separatamente.

Questa è una variante del refactoring di "Sostituisci metodo con oggetto metodo" di Fowler.

Non è un anti-pattern, ma è una mossa discutibile. Se queste funzioni non sono preziose al di fuori della loro funzione genitore, non c'è alcun vantaggio nel separarle, solo più codice. Potresti semplicemente inserire alcuni commenti nel codice che dice "ora confabuliamo qualcosa precedentemente ronzato", e avrebbe lo stesso valore in meno righe di codice.

EDIT: riformulerei la mia risposta, perché non ho usato parole molto accurate e, apparentemente, la community non l'ha accettata, in base ai downvotes. Quello che volevo veramente dire è che si dovrebbe tener conto della quantità di codice extra e di interruzioni di attenzione durante la lettura. Quando la funzione originale è abbastanza piccola (quanto piccola è basata sull'opinione pubblica), potrebbe essere più facile leggerla come un flusso di testo con alcuni commenti di aiuto, piuttosto che saltare avanti e indietro tra le funzioni interne. Potrebbe facilmente essere che una parte del codice sia più facile da leggere e comprendere rispetto a una serie di diverse funzioni.

D'altro canto, quando la funzione originale è abbastanza grande da essere interpretata come un pezzo unico, è assolutamente logico dividerlo. Il principio del raggio minore entra definitivamente in gioco qui. È solo importante capire quando la funzione è "abbastanza grande" per quello. È facile fare un simile giudizio quando quello originale è 10k LOC, come è stato commentato, ma per quanto riguarda l'esempio dell'OP, 250 LOC? Direi che sta diventando un'opinione per dare una risposta.

È certamente meglio separare ciascuna "funzione interna": le parti piccole e combinabili sono molto più facili da gestire rispetto alle "grandi palle di fango" monolitiche. Attenzione però alla terminologia: funzioni e metodi non sono la stessa cosa, dal punto di vista stilistico.

I metodi tendono a favorire un oggetto ("questo") sopra gli altri, il che limita la loro utilità per quell'oggetto. Le funzioni sono indipendenti da qualsiasi valore particolare, il che le rende più flessibili e più facili da combinare insieme.

In questo caso, la tua funzione "pippo" è chiaramente la composizione di quelle "funzioni interne", quindi potresti anche definirla in questo modo ed evitare tutto il boilerplate intermedio ("roba", "barrato", "bazzato") "," confabulated "," reticulated "," spliced "," function (stuff) {"e" return ...;} "):

var foo = [splice, reticulate, confabulate, baz, bar].reduce(compose, id);

Se non hai ancora le funzioni "componi" e "id", eccole:

var compose = function(f, g) { return function(x) { return f(g(x)); }; };
var id = function(x) { return x; };

Congratulazioni, hai appena ottenuto due parti incredibilmente utili e riutilizzabili dei presunti interni "inutili" di foo. Non posso fare molto di più senza vedere le definizioni delle funzioni "interne", ma sono sicuro che ci saranno più semplificazione, generalizzazione e riusabilità disponibili in attesa di essere scoperte.

Penso che questo tipo di codice possa essere facilmente convertito in un concatenamento. In questo modo:

foo(stuff)
{
  return    
    stuff
      .bar()
      .baz()
      .confabulate()
      .reticulate()
      .splice()        
}

Dal mio umile punto di vista, è facile da leggere, mantiene la separazione delle preoccupazioni e limita il tasso di errore (se uno dei passaggi fallisce, tutto il processo fallisce).

Non è un anti-pattern per dividere una grande funzione in funzioni più piccole.

Tuttavia dichiarare tutte queste funzioni all'interno di quella principale presenta un certo rischio: poiché le funzioni interne hanno accesso a tutte le variabili della funzione esterna, e poiché nelle variabili JavaScript sono mutabili, non è possibile garantire che le funzioni interne non modificare lo stato della funzione esterna.

In questo senso, annidandoli perdi la maggior parte dei benefici di avere piccole funzioni. Poiché tutti condividono uno stato comune mutabile, non è possibile ragionare facilmente su ciascuno in modo indipendente.

Quindi, mantieni una struttura piatta. Se vuoi nascondere le funzioni private, puoi usare il modello del modulo o una delle tante librerie di moduli JavaScript disponibili.

Sì, la divisione è buona, potrebbe essere solo per una migliore leggibilità e testabilità.

Un altro approccio potrebbe essere quello di chiamare le funzioni l'una nell'altra, si inizia con:

foo(stuff) {
   var spliced = splice(stuff);
   return spliced;
}

Poiché la funzione di splicing ha bisogno di dati reticolari, è possibile richiamare prima tale funzione:

splice(stuff) {
    var reticulated = reticulate(stuff);
    //do splicing of reticulated
    return spliced;
}

Quindi tutte le funzioni passeranno le cose e lavoreranno con i risultati che ricevono.