Questo modello è cattivo? [duplicare]

15

Ho notato che quando codifico uso spesso un pattern che chiama un metodo di classe e quel metodo chiamerà un certo numero di funzioni private nella stessa classe per fare il lavoro. Le funzioni private fanno solo una cosa. Il codice ha il seguente aspetto:

public void callThisMethod(MyObject myObject) {
    methodA(myObject);
    methodB(myObject);  // was mehtodB before
}

private void methodA(MyObject myObject) {
    //do something
}

 private void methodB(MyObject myObject) {
    //do something
}

A volte ci sono 5 o più metodi privati, in passato ho spostato alcuni dei metodi privati in un'altra classe, ma ci sono occasioni in cui fare ciò significherebbe creare complessità inutile.

Il motivo principale per cui non mi piace questo schema è che non è molto testabile, vorrei poter testare tutti i metodi individualmente, a volte renderò pubblici tutti i metodi, quindi posso scrivere test per loro.

C'è un design migliore che posso usare?

    
posta user86834 17.05.2013 - 23:16
fonte

5 risposte

25

È un buon modello. Questo è ciò a cui fa riferimento lo zio Bob quando parla di estraendo fino allo sfinimento .

Tuttavia non dovrebbe influenzare i tuoi test. Dovresti provare solo l'interfaccia pubblica. Sì, puoi riflettere la tua strada e testare i metodi privati, ma dovrai ancora testarli di nuovo quando testi i tuoi metodi pubblici, in modo che non risolva il problema percepito.

Il "problema" arriva quando stai testando un metodo privato più volte perché hai diversi metodi pubblici che li utilizzano.

A volte, in realtà più frequentemente di quanto si possa pensare, non importa. Soprattutto se stai testando per comportamento, piuttosto che provare a testare ogni riga di codice. Tutto quello che vuoi sapere è "Questo metodo pubblico fa quello che dovrebbe fare?" Non ti importa dei dettagli di implementazione.

Tuttavia, importa, quando stai testando ripetutamente lo stesso pezzo di logica complessa, perché aggiunge più test a ciascun comportamento. A quel punto, è sufficiente estrarlo in un servizio e deriderlo.

    
risposta data 17.05.2013 - 23:30
fonte
1

Ciò che terrei d'occhio è la quantità di volte in cui passi il tuo parametro iniziale object . Se tutti i metodi privati funzionano sul parametro principale, potrebbe essere un po 'un odore di codice. Per esempio. se hai:

class Foo {
    public void callThisMethod(Bar myBarObject) {
        methodA(myBarObject);
        methodB(myBarObject);
        methodC(myBarObject);
        methodD(myBarObject);
        methodE(myBarObject);

        …

        methodZ(myBarObject);

    }
}

class Bar {
    …
}

Ciò indica una bassa coesione tra i metodi di Foo con quella classe - I metodi sembrano avere più in comune con Bar che con Foo . In tal caso, considera di spostare i metodi correlati nella classe su cui stanno operando (consulta la Legge di Demeter ).

Se non è questo il caso, non c'è niente di sbagliato nell'avere metodi che fanno solo una cosa, o testare la tua applicazione attraverso i suoi metodi pubblici piuttosto che i privati interni (dato che puoi ancora "esercitare" tutti i casi d'uso di questi metodi, ma ti lascia più libero nel ridefinire questi metodi in seguito, se lo desideri).

    
risposta data 18.05.2013 - 00:34
fonte
1

Mi chiedo se stai cercando di implementare il Pattern di comando tramite metodi privati e potresti trovare il passaggio a un modello di comando più facile per quello che stai facendo.

Quello che hai ora è questo:

public void callThisMethod(MyObject myObject) {
    methodA(myObject);
    mehtodB(myObject)
}

private void methodA(MyObject myObject) {
    //do something
}

private void methodB(MyObject myObject) {
    //do something
}

Potresti implementare la stessa cosa usando i comandi e un dispatcher.

public interface Command {
    void Execute(Context c);
}

public class Context {
    // this holds shared data
}

public class Dispatcher
{
    private List<Command> _commands = new List<Command>();

    public void Add(Command cmd) 
    {
       _commands.Add(cmd);
    }

    public void Run(Context c)
    {
       foreach(Command cmd in _commands) { cmd.Execute(c); }
    }
}

Ora implementeresti i tuoi metodi privati come comandi.

public class CommandA : Command { ... }
public class CommandB : Command { ... }

Qualsiasi informazione di stato speciale è definita nella classe Context che verrà passata a ciascun comando. Per eseguire il codice devi aggiungere i comandi al dispatcher ed eseguirlo.

Dispatcher d = new Dispatcher();
d.Add(new CommandA());
d.Add(new CommandB());
d.Run(new Context());

Il vantaggio di questo approccio è che disaccoppia i comandi gli uni dagli altri, e tu usi la classe Context per far funzionare tutto insieme. È inoltre possibile aggiungere la logica a quali comandi vengono creati e quali parametri vengono utilizzati per istanziarli.

Alcuni altri suggerimenti, puoi creare singleton di comandi comunemente usati per ridurre il sovraccarico nella creazione di nuovi sempre.

Questo approccio è altamente verificabile poiché ogni comando può essere isolato.

Lo svantaggio è che una vasta collezione di comandi può diventare difficile da mantenere. È meglio creare grossi comandi che possono fare molte cose, piuttosto che semplici piccoli comandi in grande numero.

    
risposta data 18.05.2013 - 04:02
fonte
1

Credo che lo schema in questione sia noto come metodo composto ed è menzionato nei modelli Kent Becks Smalltalk Best practice [1]. Inoltre, egli menziona che ogni metodo fa una cosa identificabile e che i metodi chiamati all'interno di un metodo dovrebbero essere tutti allo stesso livello di astrazione.

[1] link

    
risposta data 03.06.2013 - 18:35
fonte
0

Per me questa sembra una cattiva pratica per due motivi.

Non è chiaro come questi metodi dipendano l'uno dall'altro. Se c'è un ordine di aderire rigorosamente a questo ordine, questo codice dovrebbe essere riportato nel codice. Altrimenti un accoppiamento temporale nascosto ( vedi l'articolo G31 in Pulisci codice ). Rendere ovvio l'accoppiamento può essere ottenuto restituendo un oggetto da ogni metodo e passandolo al metodo successivo.

Se questi metodi non dipendono realmente l'uno dall'altro, si dovrebbe introdurre un'interfaccia con un singolo metodo. Le diverse implementazioni devono essere mantenute in una raccolta in un campo della classe proprietaria di callThisMethod e tale metodo deve solo scorrere attraverso quella raccolta e richiamare il metodo degli elementi di raccolta. In questo modo stai aderendo maggiormente al principio aperto aperto , perché aggiungere un'altra funzionalità è solo questione di configurare un'altra implementazione dell'interfaccia da conservare in quell'elenco e non modificare la classe proprietaria di callThisMethod

    
risposta data 04.06.2013 - 09:55
fonte

Leggi altre domande sui tag