2008-12-11 12 views
8

Ho appena finito di guardare il video del codice pulito di Google su YouTube (vedere link, primo articolo) sulla rimozione delle istruzioni if dal codice e utilizzare invece il polimorfismo.Come si dovrebbe ridefinire questo condizionale per utilizzare il polimorfismo?

Dopo aver visto il video che ho dato un'occhiata al codice che stavo scrivendo, prima di guardare il video e ho notato alcuni posti in cui ho potuto utilizzare questo metodo, soprattutto luoghi in cui lo stesso tipo di logica è stato implementato molte volte. Quindi un esempio:

Ho un codice come questo.

public int Number 
{ 
    get 
    { 
     string returnValue; 
     if (this.internalTableNumber == null) 
      returnValue = this.RunTableInfoCommand(internalTableName, 
                TableInfoEnum.TAB_INFO_NUM); 
     else 
      returnValue = this.RunTableInfoCommand(internalTableNumber.Value, 
                TableInfoEnum.TAB_INFO_NUM); 
     return Convert.ToInt32(returnValue); 
    } 
} 

Cosa RunTableInfoCommand non è veramente importante, ma la cosa principale è che non ho molte proprietà con esattamente lo stesso if statments l'unica cosa che cambia è il TableInfoEnum.

Mi chiedevo se qualcuno potrebbe aiutarmi a refactarlo in modo che faccia ancora la stessa cosa ma senza alcuna istruzione if?

+0

Perché il voto negativo? –

+1

Perché in origine hai detto "usa il polimorfismo", poi hai detto "oh no, non ho bisogno di polimorfismo" - e avevo già speso una buona parte di quello che pensavo fosse tempo sprecato per rispondere a una domanda che non ti interessava.Ora che "usa il polimorfismo" è tornato, ho rimosso il voto negativo e non ha annullato la mia risposta – tvanfosson

+1

Scusa, non volevo perdere tempo a nessuno, havin 'uno di quei giorni, sono ancora interessato a vedere come la gente lo farebbe comunque. Di nuovo molto dispiaciuto. –

risposta

8

Solo una nota di avvertimento qui dopo aver visto alcune di queste risposte (tecnicamente corrette), semplicemente sbarazzarsi di una dichiarazione di If non dovrebbe essere il vostro unico scopo, l'obiettivo dovrebbe essere quello di rendere il vostro codice estensibile, mantenibile e semplice, se quello significa liberarsi di un'istruzione if, ottimo, ma non dovrebbe essere un obiettivo in sé.

Nell'esempio di codice che hai fornito, e senza sapere di più sulla tua app, e supponendo che non stai andando a estendere molti test passati per un valore nullo, penso che un If (o forse anche un ternario) sia il più soluzione manutenibile per essere perfettamente sinceri.

+0

Sì, questo business di cercare di sostituire tutte le tue if-statement con un approccio polimorfo è un po 'pazzo. – BobbyShaftoe

+0

Sì, ho rilasciato ora non è la cosa più intelligente da fare in questo caso, ma mi piacerebbe ancora vedere come la gente lo attaccherebbe. –

0

Vorrei prendere in considerazione il trasferimento del recupero del valore di ritorno in un'altra classe che potrebbe essere iniettata in fase di esecuzione.

public class Thing 
{ 
    public IValueFetcher ValueFetcher { get; set; } 

    public int Number 
    { 
    get 
    { 
     return this.ValueFetcher.GetValue<int>(/* parameters to identify the value to fetch */); 
    } 
    } 
} 

che avrebbe preso cura di un sacco di codice ripetitivo e ridurre la dipendenza dalla fonte del valore da un'interfaccia.

Penso che a un certo punto è probabile avere un'istruzione if, poiché è ancora necessario decidere quale versione di RunTableInfoCommand si desidera chiamare.

4

Realizzerete effettivamente qualcosa come il modello di strategia. Inizia definendo una super classe, chiamiamola AbstractTableInfoCommand. Questa classe può essere astratta ma deve specificare un metodo chiamato runTableInfoCommand().

È quindi possibile definire diverse sottoclassi che implementano ciascun metodo runTableInfoCommand(). La tua classe, quella con la proprietà Number, avrà quindi una nuova proprietà di tipo AbstractTableInfoCommand (chiamiamola tableInfoCommand) che verrà istanziata a una delle sottoclassi concrete di AbstractTableInfoCommand.

Il codice sarà quindi:

public int Number 
    { 
     get 
     { 

      return this.tableInfoCommand.runTableInfoCommand(); 
     } 
    } 

in modo da poter creare un NullTableInfoCommand e SomeOtherTableInfoCommand ecc Il vantaggio è che, se avete qualche nuova condizione per la restituzione di un tableinfocommand quindi si aggiunge una nuova classe, piuttosto che modificare questo codice

Detto questo, non tutte le situazioni sono necessariamente giuste per questo schema. Quindi rende il codice più estensibile, ma se si è in una situazione che non richiede tale estensione, è eccessivo.

0

presumo internTableName e InternalTableNumber sono una sorta di valore per la stessa cosa.Perché non avvolgetelo all'interno di una classe e passare un'istanza di quella classe di this.RunTableInfoCommand in questo modo:

public int Number 
     { 
      get 
      { 
       string returnValue; 
       internalTableClass myinstance(parameters); 
       return Convert.ToInt32(this.RunTableInfoCommand(myinstance, TableInfoEnum.TAB_INFO_NUM)); 
      } 
     } 

se si vuole ancora utilizzare il polimorfismo, allora, è possibile farlo in quella classe da sovraccarico, per esempio un giveInternalTableIdentifier che restituisce il numero o il nome

il codice qui potrebbe quindi simile a questa:

public int Number 
     { 
      get 
      { 
       string returnValue; 
       internalTableClass myinstance(parameters); 
       return Convert.ToInt32(this.RunTableInfoCommand(myinstance.giveInternalTableIdentifier, TableInfoEnum.TAB_INFO_NUM)); 
      } 
     } 

e il codice per l'internalTableClass sarà banale (utilizzando: internalAbstractTableClass, e due classi che ereditano da essa, una dando il nome e l'altro il numero)

0

EDIT 2 Come risolverei davvero il problema.

Farei InternalTableNumber una proprietà che è pigro caricato. Se non è disponibile, dovrei cercarlo tramite InternalTableName. Quindi utilizzerei sempre la proprietà InternalTableNumber per i miei metodi.

private int? internalTableNumber; 
private int InternalTableNumber 
{ 
    get 
    { 
     if (!internalTableNumber.HasValue) 
     { 
      internalTableNumber = GetValueFromTableName(internalTableName); 
     } 
     return internalTableNumber; 
    } 
    set 
    { 
     internalTableNumber = value; 
    } 
} 

public int Number 
{ 
    get 
    { 
     string value = this.RunTableInfoCommand(InternalTableNumber, 
               TableInfoEnum.TAB_INFO_NUM); 
     return Convert.ToInt32(value); 
    } 
} 

EDIT Uso del polimorfismo ...

Supponiamo che la classe corrente è chiamato Foo, quindi vorrei refactoring in due classi, FooWithName e FooWithNumber. FooWithName sarebbe la classe che useresti quando avrai il nome della tabella e FooWithNumber sarà la classe da usare quando avrai il numero della tabella. Scriverò quindi ogni classe con un metodo numerico - in realtà scriverò anche un'interfaccia IFoo che implementi tutti in modo che possano essere usati in modo intercambiabile.

public interface IFoo 
{ 
    int Number { get; }| 

} 

public class FooWithName : IFoo 
{ 
    private string tableName; 
    public FooWithName(string name) 
    { 
     this.tableName = name; 
    } 

    public int Number 
    { 
     get { return this.RunTableInfoCommand(this.tableName, 
             TableInfoEnum.TAB_INFO_NUM); 
    } 

    ... rest of class, including RunTableInfoCommand(string,int); 
} 

public class FooWithNumber : IFoo 
{ 
    private int tableNumber; 
    public FooWithNumber(int number) 
    { 
     this.tableNumber = number; 
    } 

    public int Number 
    { 
     get { return this.RunTableInfoCommand(this.tableNumber, 
             TableInfoEnum.TAB_INFO_NUM); 
    } 

    ... rest of class, including RunTableInfoCommand(int,int); 
} 

La si usa come così:

IFoo foo; 

if (tableNumber.HasValue) 
{ 
    foo = new FooWithNumber(tableNumber.Value); 
} 
else 
{ 
    foo = new FooWithName(tableName); 
} 

int number = foo.Number; 

Ovviamente, se non avete un sacco di costrutti then-else if-nella classe esistente, questa soluzione in realtà non migliorarlo tanto. Questa soluzione crea IFoo usando il polimorfismo e quindi usa solo i metodi dell'interfaccia senza preoccuparsi dell'implementazione. Questo potrebbe facilmente essere esteso per ereditare un'implementazione comune di RunTableCommand (int) in una classe astratta che eredita IFoo ed è la classe base di FooWithNum e FooWithName.

0

avrei refactoring così:

table = this.internalTableNumber == null ? internalTableName : internalTableNumber.Value; 
return Convert.ToInt32(this.RunTableInfoCommand(table, TableInfoEnum.TAB_INFO_NUM)); 

Amore l'operatore ternario.

1

Sento che il tuo codice è perfettamente a posto. Leggibile. Semplice. (E spero funzioni). Se questo blocco di codice viene ripetuto n volte, è necessario rimuovere la duplicazione applicando il metodo Extract.

Il refactoring che si indica è destinato a sostituire casi di switch ricorrenti .. non è semplice se le dichiarazioni come nell'esempio. Replace Conditional With Polymorphism. Ricorda la "cosa più semplice che funziona" .. che significa il numero minimo di classi e metodi necessari per portare a termine il lavoro.

+0

Se ogni proprietà eseguisse lo stesso se/then/else sarebbe comunque un buon candidato per il polimorfismo. – tvanfosson

+0

Perché non isolare il costrutto if in un metodo, che ogni proprietà può quindi chiamare? La linea di tag refactoring RCWP è "Hai un condizionale che sceglie un comportamento diverso a seconda del tipo di oggetto." - IMHO .. non si applica qui. – Gishu

+0

Questo è quello che ho fatto ora, non stavo davvero pensando quando l'ho buttato qui. –

0

In questo caso, optare per un refactoring che implichi il polimorfismo potrebbe essere eccessivo (a seconda di quali altri vantaggi si potrebbero ottenere dal polimorfismo). In questo caso, aggiungere un semplice overload che incapsula la logica di cui RunTableInfoCommand() da chiamare potrebbe essere in ordine.

Dal RunTableInfoCommand(), internalTableNumber, e internalTableName tutti sembrano essere membri della stessa stessa classe, una bella, facile refactoring potrebbe essere quella di aggiungere un sovraccarico di RunTableInfoCommand() che prende semplicemente un valore TableInfoEnum e fa la logica di capire quali altri RunTableInfoCommand() sovraccarico deve essere chiamato:

private string RunTableInfoCommand(TableInfoEnum infoEnum) 
{ 
    if (this.internalTableNumber == null) { 
     return this.RunTableInfoCommand(internalTableName, infoEnum); 
    } 

    return this.RunTableInfoCommand(internalTableNumber.Value, infoEnum); 
} 

Poi i numerosi siti di chiamata che hanno la stessa logica if decisione può essere crollato a:

returnValue = this.RunTableInfoCommand(TableInfoEnum.TAB_INFO_NUM);  
             // or whatever enum is appropriate 
0

come potrei refactoring queste particolari dichiarazioni se utilizzare il polimorfismo?

Beh ... non lo farei. Non hai bisogno di polimorfismo per eliminare le dichiarazioni if.

Si noti che il se affermazioni come non e 'cattivo' di per sé, l'if sono causati dalla scelta di rappresentanza in cui

(internalTableNumber == null) ==> 
    internalTableName else internalTableNumber.Value 

Questa associazione implica una classe mancante, vale a dire che avrebbe più senso avere una classe InternalTable che sia proprietaria di InternalTableName e di InternalTablenumber, poiché questi due valori sono fortemente associati alla regola di controllo null.

  • Questa nuova classe potrebbe fornire una proprietà TableNumber che ha eseguito l'internalTableNumber == null check
  • e la RunTableInfoCommand potrebbe prendere un'istanza InternalTable come parametro (ma non deve, vedere la voce successiva).
  • Ma ancora meglio, la nuova classe dovrebbe avere una facciata per il metodo RunTableInfoCommand (che è presumibilmente statico) che ha eseguito la conversione dei numeri interi.

Supponendo che l'istanza InternalTable si chiama iTable, il codice refactoring di preoccupazione sarebbe quindi simile:

public int Number 
{ 
    get 
    { 
     return iTable.RunTableInfoCommand(TableInfoEnum.TAB_INFO_NUM); 
    } 
} 

Ciò incapsulare il nulla-check nella classe InternalTable. La modifica della firma RunTableInfoCommand originale è facoltativa.

Nota che questo è non "sostituire le istruzioni if ​​con polimorfismo", ma elimina le istruzioni if ​​dalla classe di consumo tramite l'incapsulamento.

+0

si potrebbe semplicemente fare una funzione per nascondere il controllo null, ma la firma di questa funzione sarebbe un sottoinsieme dello stato dell'oggetto, che potrebbe implicare logicamente una nuova classe ... –

Problemi correlati