2008-12-11 9 views
8

Ich habe gerade das Google Clean Code-Video auf YouTube (siehe link, erster Artikel) über das Entfernen von if-Anweisungen aus Ihrem Code und die Verwendung von Polymorphie abgeschlossen.Wie würden Sie diese Bedingung für die Verwendung von Polymorphie umgestalten?

Nachdem ich das Video angeschaut habe, habe ich mir einen Code angeschaut, den ich geschrieben habe, bevor ich das Video angeschaut habe und einige Stellen bemerkt habe, an denen ich diese Methode verwenden konnte, hauptsächlich Orte, an denen die selbe Logik mehrfach implementiert wurde. Also ein Beispiel:

Ich habe einen Code wie diesen.

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); 
    } 
} 

Was RunTableInfoCommand tut, ist nicht wirklich wichtig, aber die Hauptsache ist, dass ich mit vielen Eigenschaften haben genau das gleiche if statments das einzige, was die TableInfoEnum verändert ist.

Ich frage mich, ob jemand mir helfen könnte, dies zu refaktorieren, so dass es immer noch das Gleiche tut, aber ohne irgendwelche if Aussagen?

+0

Warum die Abstimmung unten? –

+1

Weil du ursprünglich gesagt hast "nutze Polymorphismus", dann sagte "Oh nein, ich brauche keinen Polymorphismus" - und ich hatte schon eine ganze Menge von dem ausgegeben, was ich für Zeitverschwendung hielt, um eine Frage zu beantworten, die dir egal war.Nun, da "polymorphism" zurück ist, habe ich die down vote entfernt und meine Antwort gelöscht. – tvanfosson

+1

Entschuldigung, ich wollte die Zeit nicht verschwenden, an einem dieser Tage bin ich immer noch interessiert zu sehen, wie Leute würden es aber tun. Wiederum sehr leid. –

Antwort

8

Nur ein warnender Hinweis hier einige dieser (technisch korrekten) Antworten zu sehen, nur eine If-Anweisung loszuwerden sollte nicht Ihr einziges Ziel sein, sollte das Ziel sein, Ihren Code erweiterbar, wartbar und einfach zu machen, wenn das bedeutet, eine if-Aussage loszuwerden, groß, aber es sollte kein Ziel an sich sein.

In dem Codebeispiel, das Sie angegeben haben, und ohne mehr über Ihre App zu wissen, und vorausgesetzt, Sie werden nicht viel nach dem Test für einen Nullwert erweitern, denke ich ein If (oder vielleicht sogar ein ternäres) ist mehr Wartbare Lösung, um vollkommen ehrlich zu sein.

+0

Ja, ich versuche, all Ihre if-Statements durch einen polymorphen Ansatz zu ersetzen, ist ein bisschen verrückt. – BobbyShaftoe

+0

Ja, ich habe jetzt veröffentlicht, es ist nicht die klügste Sache in diesem Fall, aber ich würde immer noch gerne sehen, wie die Leute es angreifen würden. –

0

Ich würde vielleicht in Erwägung ziehen, das Abrufen des Rückgabewerts an eine andere Klasse zu übergeben, die zur Laufzeit injiziert werden könnte.

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

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

, die sich um eine Menge von sich wiederholenden Code nehmen und Ihre Abhängigkeit von der Quelle des Wertes an eine Schnittstelle zu reduzieren.

Ich denke, dass Sie wahrscheinlich eine if-Anweisung haben werden, da Sie immer noch entscheiden müssen, welche Version von RunTableInfoCommand Sie aufrufen möchten.

4

Sie werden tatsächlich etwas wie das Strategie-Muster implementieren. Beginnen Sie mit der Definition einer Superklasse, lassen Sie sie AbstractTableInfoCommand aufrufen. Diese Klasse kann abstrakt sein, muss jedoch eine Methode namens runTableInfoCommand() angeben.

Sie können dann mehrere Unterklassen definieren, die jeweils die Methode runTableInfoCommand() implementieren. Ihre Klasse, die mit der Number-Eigenschaft, hat dann eine neue Eigenschaft vom Typ AbstractTableInfoCommand (lasst es tableInfoCommand nennen), die in einer der konkreten Unterklassen von AbstractTableInfoCommand instanziiert wird.

Der Code wird dann sein:

public int Number 
    { 
     get 
     { 

      return this.tableInfoCommand.runTableInfoCommand(); 
     } 
    } 

So können Sie eine erstellen NullTableInfoCommand und SomeOtherTableInfoCommand usw. Der Vorteil ist, dass, wenn Sie einige neue Bedingung für die Rückgabe eines tableinfocommand dann fügen Sie eine neue Klasse, anstatt bearbeiten dieser Code.

Nichtsdestoweniger ist nicht jede Situation notwendig für dieses Muster richtig. Es macht also mehr erweiterbaren Code, aber wenn Sie in einer Situation sind, die diese Erweiterbarkeit nicht erfordert, ist es übertrieben.

0

Ich nehme an, internTableName und InternalTableNumber sind eine Art von Wert für die gleiche Sache.Warum es nicht innerhalb einer Klasse einpacken und passieren eine Instanz dieser Klasse zu this.RunTableInfoCommand wie so:

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

wenn Sie noch Polymorphismus verwenden dann möchten, können Sie dies in dieser Klasse zum Beispiel durch Überlastung eine giveInternalTableIdentifier, dass gibt die Nummer oder den Namen

hier den Code dann wie folgt aussehen könnte:

für die internalTableClass wird (unter Verwendung trivial
public int Number 
     { 
      get 
      { 
       string returnValue; 
       internalTableClass myinstance(parameters); 
       return Convert.ToInt32(this.RunTableInfoCommand(myinstance.giveInternalTableIdentifier, TableInfoEnum.TAB_INFO_NUM)); 
      } 
     } 

und der Code: internalAbstractTableClass und zwei Klassen, die von ihm erben, ein Geben Sie den Namen, und die andere die Nummer)

0

EDIT 2 Wie würde ich wirklich das Problem lösen.

Ich würde InternalTableNumber eine Eigenschaft machen, die Lazy geladen ist. Wenn es nicht verfügbar ist, würde ich es über den InternalTableName nachschlagen. Dann würde ich immer nur die InternalTableNumber-Eigenschaft für meine Methoden verwenden.

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 Polymorphismus ...

Nehmen wir an, dass die aktuelle Klasse Foo genannt, dann würde ich es in zwei Klassen Refactoring, FooWithName und FooWithNumber. FooWithName wäre die Klasse, die Sie verwenden würden, wenn Sie den Tabellennamen haben, und FooWithNumber wäre die Klasse, die Sie verwenden möchten, wenn Sie die Tabellennummer haben. Ich würde dann jede Klasse mit einer Number-Methode schreiben - eigentlich schreibe ich auch eine Schnittstelle IFoo, die jeweils so implementiert ist, dass sie austauschbar verwendet werden können.

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); 
} 

Das würden Sie es verwenden, wie so:

IFoo foo; 

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

int number = foo.Number; 

Offensichtlich, es sei denn Sie eine Menge der if-then-else-Konstrukte in Ihrer bestehenden Klasse, ist diese Lösung nicht tatsächlich verbessern viel. Diese Lösung erstellt IFoo mit Polymorphie und verwendet dann nur die Interface-Methoden, ohne sich um die Implementierung zu kümmern. Dies könnte leicht erweitert werden, um eine allgemeine Implementierung von RunTableCommand (int) in einer abstrakten Klasse zu erben, die IFoo erbt und die Basisklasse für FooWithNum und FooWithName ist.

0

Ich würde es thusly Refactoring:

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

Die Liebe der ternäre Operator.

1

Ich fühle, dass Ihr Code vollkommen in Ordnung ist. Lesbar. Einfach. (Und ich hoffe, es funktioniert). Wenn dieser Codeblock sich n-mal wiederholt, müssen Sie die Duplizierung durch Anwenden der Extract-Methode entfernen.

Das Refactoring, das Sie angeben, soll wiederkehrende Schalterfälle ersetzen .. nicht einfache if-Anweisungen wie in Ihrem Beispiel. Replace Conditional With Polymorphism. Erinnern Sie sich an die "einfachste Sache, die funktioniert". Dies bedeutet die minimale Anzahl von Klassen und Methoden, die erforderlich sind, um die Arbeit zu erledigen.

+0

Wenn jede Eigenschaft die gleiche ausgeführt würde, wenn/dann/sonst wäre es immer noch ein guter Kandidat für Polymorphie. – tvanfosson

+0

Warum isolieren Sie das if-Konstrukt nicht in eine Methode, die jede Eigenschaft dann aufrufen kann? Die RCWP-Refactoring-Tag-Zeile lautet "Sie haben eine Bedingung, die abhängig vom Typ eines Objekts ein anderes Verhalten auswählt." - IMHO .. es gilt hier nicht. – Gishu

+0

Das habe ich jetzt gemacht, ich habe nicht wirklich daran gedacht, als ich das hier hochwarf. –

0

Ein Refactoring, das Polymorphismus beinhaltet, könnte in diesem Fall übertrieben sein (abhängig davon, welche anderen Vorteile Sie aus dem Polymorphismus herausholen könnten). In diesem Fall kann eine einfache Überladung hinzugefügt werden, die die Logik einkoppelt, von der RunTableInfoCommand() aufgerufen werden kann.

Seit RunTableInfoCommand(), internalTableNumber und internalTableName scheinen alle Mitglieder derselben gleichen Klasse, ein schönes, leicht Refactoring könnte sein, um eine Überlastung der RunTableInfoCommand() hinzuzufügen, die einfach einen TableInfoEnum Wert annimmt und tut die Logik herauszufinden, welche anderen RunTableInfoCommand() Überlastung muss aufgerufen werden:

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

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

Dann werden die zahlreichen Call-Sites, die die gleiche if Entscheidungslogik haben kann zusammengeklappt werden:

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

Wie würde ich diese speziellen if-Anweisungen umpolieren, um Polymorphie zu verwenden?

Nun ... ich würde nicht. Sie benötigen keinen Polymorphismus, um die if-Anweisungen zu eliminieren.

Beachten Sie, dass die if-Anweisungen nicht ‚schlecht‘ per se, die if-Anweisungen werden durch die Darstellung Wahl verursacht, wo

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

Dieser Verein eine fehlende Klasse impliziert, dh es mehr Sinn machen würde, eine InternalTable-Klasse besitzen, die internalTableName und internalTablenumber besitzt, da diese beiden Werte von der Nullprüfungsregel stark verknüpft sind.

  • könnte diese neue Klasse eine Eigenschaft tablenumber bereitzustellen, die die internalTableNumber == null Prüfung
  • und den RunTableInfoCommand führten eine InternalTable Instanz als Parameter übernehmen könnte (aber es muss nächsten Punkt nicht finden).
  • Aber noch besser, die neue Klasse sollte eine Fassade für die RunTableInfoCommand-Methode (die vermutlich statisch ist), die die Integer-Konvertierung durchgeführt haben.

die InternalTable Instanz Angenommen iTable genannt wird, würde der Code Refactoring Sorge dann wie folgt aussehen:

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

Dies würde den Null-Check in der InternalTable Klasse kapselt. Das Ändern der ursprünglichen RunTableInfoCommand-Signatur ist optional.

Beachten Sie, dass dies nicht "ersetzen die if-Anweisungen mit Polymorphismus", aber es beseitigt die if-Anweisungen von der konsumierenden Klasse über Kapselung.

+0

Sie könnten einfach eine Funktion zum Verbergen des Null-Checks erstellen, aber die Signatur dieser Funktion wäre eine Teilmenge des Zustands des Objekts, was logisch eine neue Klasse implizieren könnte ... –

Verwandte Themen