Pattern per una classe che fa una sola cosa

Esiste un modello chiamato Method Object in cui si calcola un singolo metodo di grandi dimensioni con un sacco di variabili temporanee / argomenti in una classe separata. Lo fai invece di estrarre parti del metodo in metodi separati perché hanno bisogno di accedere allo stato locale (i parametri e le variabili temporanee) e non puoi condividere lo stato locale usando le variabili di istanza perché sarebbero locali a quel metodo (e i metodi estratti da esso) solo e andrebbero inutilizzati dal resto dell'oggetto.

Quindi, il metodo diventa una sua classe, i parametri e le variabili temporanee diventano variabili di istanza di questa nuova classe, quindi il metodo viene suddiviso in metodi più piccoli della nuova classe. La classe risultante di solito ha solo un singolo metodo di istanza pubblico che esegue l'attività incapsulata dalla classe.

Direi che la classe in questione (utilizzando un singolo punto di ingresso) è ben progettata. È facile da usare ed estendere. Piuttosto SOLID.

Stessa cosa per no "externally recognizable" state . È un buon incapsulamento.

Non vedo alcun problema con codice come:

var doer = new StuffDoer(initialParams);
var result = doer.Calculate(extraParams);

Da un principi SOLID prospettiva la risposta di jgauffin ha senso. Tuttavia, non dovresti dimenticare i principi generali di progettazione, ad es. nascondere le informazioni .

Vedo diversi problemi con l'approccio dato:

• Come hai sottolineato tu stesso, le persone non si aspettano di utilizzare la parola chiave "nuova", quando l'oggetto creato non gestisce nessuno stato . Il tuo design riflette la sua intenzione. Le persone che utilizzano la tua classe potrebbero confondersi su quale stato gestisce e se le successive chiamate al metodo potrebbero comportare un comportamento diverso.
• Dal punto di vista della persona che usa la classe, lo stato interiore è ben nascosto, ma quando vuoi apportare modifiche alla classe o semplicemente comprenderla, stai rendendo le cose più complesse. I hanno già scritto molto sui problemi che vedo con i metodi di suddivisione solo per renderli più piccoli , specialmente quando si sposta lo stato sull'ambito della classe. Stai modificando il modo in cui la tua API dovrebbe essere utilizzata solo per avere funzioni più piccole! che a mio avviso sta decisamente prendendo troppo.

Alcuni riferimenti correlati

Forse un punto di discussione principale sta nella misura in cui estendere il principio di responsabilità singola . "Se lo porti a quell'estremo e costruisci classi che hanno una ragione per esistere, potresti finire con un solo metodo per classe. Ciò causerebbe una grande dispersione di classi anche per i processi più semplici, causando sistema difficile da capire e difficile da modificare. "

Un altro riferimento pertinente relativo a questo argomento: "Dividi i tuoi programmi in metodi che eseguono un'attività identificabile. Mantieni tutte le operazioni in un metodo su lo stesso livello di astrazione ." - Kent Beck La chiave qui è "lo stesso livello di astrazione". Ciò non significa "una cosa", poiché viene spesso interpretata. Questo livello di astrazione dipende interamente dal contesto per il quale stai progettando.

Quindi qual è l'approccio corretto?

Senza conoscere il tuo caso d'uso concreto è difficile dirlo. C'è uno scenario in cui a volte (non spesso) uso un approccio simile. Quando voglio elaborare un set di dati, senza voler rendere questa funzionalità disponibile per l'intero ambito di classe. Ho scritto un post sul blog, come lambdas può migliorare ulteriormente l'incapsulamento . I hanno anche avviato una domanda sull'argomento qui su Programmers . Quello che segue è un ultimo esempio di dove ho usato questa tecnica.

new TupleList<Key, int>
{
    { Key.NumPad1, 1 },
            ...
    { Key.NumPad3, 16 },
    { Key.NumPad4, 17 },
}
    .ForEach( t =>
    {
        var trigger = new IC.Trigger.EventTrigger(
                        new KeyInputCondition( t.Item1, KeyInputCondition.KeyState.Down ) );
        trigger.ConditionsMet += () => AddMarker( t.Item2 );
        _inputController.AddTrigger( trigger );
    } );

Poiché il codice molto "locale" all'interno di ForEach non viene riutilizzato da nessun'altra parte, posso semplicemente tenerlo nella posizione esatta in cui è rilevante. Delineare il codice in modo tale che il codice che si affida a vicenda è strongmente raggruppato insieme rende più leggibile a mio parere.

Possibili alternative

• In C # è possibile utilizzare invece i metodi di estensione. Quindi agisci sull'argomento direttamente passerai a questo metodo 'una cosa'.
• Verifica se questa funzione in realtà non appartiene a un'altra classe.
• Rendi una funzione statica in una classe statica . Questo è probabilmente l'approccio più appropriato, come si riflette anche nelle API generali a cui ti sei riferito.

Direi che è abbastanza buono, ma che la tua API dovrebbe essere ancora un metodo statico su una classe statica, poiché è ciò che l'utente si aspetta. Il fatto che il metodo statico usi new per creare il tuo oggetto helper e fare il lavoro è un dettaglio di implementazione che dovrebbe essere nascosto a chiunque lo stia chiamando.

new StuffDoer().Start(initialParams);

C'è un problema con gli sviluppatori clueless che potrebbero usare un'istanza condivisa e usarla

1. più volte (ok se l'esecuzione precedente (forse parziale) non compromette l'esecuzione successiva)
2. da più thread (non OK, hai detto esplicitamente che ha stato interno).

Quindi questo richiede una documentazione esplicita che non sia thread-safe e che sia leggero (la creazione è veloce, non lega risorse esterne né molta memoria) che è possibile istanziare al volo.

Nasconderlo in un metodo statico aiuta con questo, perché quindi il riutilizzo dell'istanza non avviene mai.

Se ha alcune costose inizializzazioni, potrebbe (non è sempre) essere utile prepararlo a inizializzarlo una volta e utilizzarlo più volte creando un'altra classe che conterrebbe solo lo stato e renderla clonabile. L'inizializzazione creerebbe uno stato che verrebbe memorizzato in doer . start() lo clonerà e lo passerà ai metodi interni.

Ciò consentirebbe anche altre cose come lo stato di esecuzione parziale permanente. (Se impiegano fattori estesi e lunghi, ad esempio errori di fornitura di energia elettrica, potrebbero interrompere l'esecuzione.) Ma questi elementi di fantasia aggiuntivi di solito non sono necessari, quindi non ne vale la pena.

Oltre a la mia risposta più elaborata e personalizzata , ritengo che la seguente osservazione meriti una risposta separata.

Ci sono indicazioni che potresti seguire l'anti-pattern Poltergeists .

Poltergeists are classes with very limited roles and effective life cycles. They often start processes for other objects. The refactored solution includes a reallocation of responsibilities to longer-lived objects that eliminate the Poltergeists.

Symptoms And Consequences

• Redundant navigation paths.
• Transient associations.
• Stateless classes.
• Temporary, short-duration objects and classes.
• Single-operation classes that exist only to “seed” or “invoke” other classes through temporary associations.
• Classes with “control-like” operation names such as start_process_alpha.

Refactored Solution

Ghostbusters solve Poltergeists by removing them from the class hierarchy altogether. After their removal, however, the functionality that was “provided” by the poltergeist must be replaced. This is easy with a simple adjustment to correct the architecture.

Sono disponibili diversi modelli sul P del catalogo EAA di Martin Fowler ;

1. Transaction Script : un po 'simile all'approccio che hai citato.
2. Sostituisci metodo w / Metodo oggetto : sembra un buon approccio anche come @anon menzionato.