Machen Sie niemals öffentliche Mitglieder virtuell / abstrakt - wirklich?

20

In den 2000er Jahren sagte mir ein Kollege, es sei ein Anti-Pattern, öffentliche Methoden virtuell oder abstrakt zu machen.

Zum Beispiel hielt er eine Klasse wie diese für nicht gut designt:

public abstract class PublicAbstractOrVirtual
{
  public abstract void Method1(string argument);

  public virtual void Method2(string argument)
  {
    if (argument == null) throw new ArgumentNullException(nameof(argument));
    // default implementation
  }
}

Das hat er gesagt

  • Der Entwickler einer abgeleiteten Klasse, die implementiert Method1und überschreibt, Method2muss die Argumentvalidierung wiederholen.
  • Falls der Entwickler der Basisklasse beschließt, etwas um den anpassbaren Teil von Method1oder Method2später hinzuzufügen , kann er dies nicht tun.

Stattdessen schlug mein Kollege diesen Ansatz vor:

public abstract class ProtectedAbstractOrVirtual
{
  public void Method1(string argument)
  {
    if (argument == null) throw new ArgumentNullException(nameof(argument));
    this.Method1Core(argument);
  }

  public void Method2(string argument)
  {
    if (argument == null) throw new ArgumentNullException(nameof(argument));
    this.Method2Core(argument);
  }

  protected abstract void Method1Core(string argument);

  protected virtual void Method2Core(string argument)
  {
    // default implementation
  }
}

Er sagte mir, dass es genauso schlimm ist, öffentliche Methoden (oder Eigenschaften) virtuell oder abstrakt zu machen, wie Felder öffentlich zu machen. Wenn Sie Felder in Eigenschaften einschließen, können Sie bei Bedarf den Zugriff auf diese Felder später abfangen. Dasselbe gilt für öffentliche virtuelle / abstrakte Member: Wenn diese wie in der ProtectedAbstractOrVirtualKlasse gezeigt verpackt werden, kann der Basisklassenentwickler alle Aufrufe abfangen, die an die virtuellen / abstrakten Methoden gesendet werden.

Aber ich sehe das nicht als Designrichtlinie. Selbst Microsoft folgt dem nicht: Schauen Sie sich die StreamKlasse an, um dies zu überprüfen.

Was halten Sie von dieser Richtlinie? Ergibt es irgendeinen Sinn oder denken Sie, dass es die API überkompliziert?

Peter Perot
quelle
5
Das Erstellen von Methoden virtual ermöglicht optionales Überschreiben. Ihre Methode sollte wahrscheinlich öffentlich sein, da sie möglicherweise nicht überschrieben wird. Wenn Sie Methoden abstract erstellen, müssen Sie diese überschreiben. Sie sollten es wahrscheinlich sein protected, weil sie in einem publicKontext nicht besonders nützlich sind .
Robert Harvey
4
Dies protectedist besonders nützlich, wenn Sie private Mitglieder der abstrakten Klasse abgeleiteten Klassen aussetzen möchten. Auf jeden Fall bin ich nicht besonders besorgt über die Meinung Ihres Freundes; Wählen Sie den Zugriffsmodifikator, der für Ihre spezielle Situation am sinnvollsten ist.
Robert Harvey
4
Ihr Kollege hat sich für das Template Method Pattern ausgesprochen . Es kann Anwendungsfälle für beide Arten geben, abhängig davon, wie stark die beiden Methoden voneinander abhängig sind.
Greg Burghardt
8
@ GregBurghardt: Klingt so, als würde der OP-Kollege vorschlagen, immer das Muster der Vorlagenmethode zu verwenden, unabhängig davon, ob dies erforderlich ist oder nicht. Das ist eine typische Überbeanspruchung von Mustern - wenn man einen Hammer hat, fängt früher oder später jedes Problem an, wie ein Nagel auszusehen ;-)
Doc Brown
3
@PeterPerot: Ich hatte nie ein Problem, mit einfachen DTOs mit nur öffentlichen Feldern zu beginnen, und wenn sich herausstellte, dass für ein solches DTO Mitglieder mit Geschäftslogik erforderlich waren, sollten Sie sie in Klassen mit Eigenschaften umgestalten. Sicherlich ist es anders, wenn man als Bibliotheksverkäufer arbeitet und darauf achten muss, keine öffentlichen APIs zu ändern. Selbst wenn ein öffentliches Feld in eine öffentliche Eigenschaft mit demselben Namen umgewandelt wird, kann dies Probleme verursachen.
Doc Brown

Antworten:

30

Sprichwort

dass es ein Anti-Pattern ist, öffentliche Methoden virtuell oder abstrakt zu machen, weil der Entwickler einer abgeleiteten Klasse, die Methode1 implementiert und Methode2 überschreibt, die Argumentvalidierung wiederholen muss

vermischt Ursache und Wirkung. Es wird davon ausgegangen, dass für jede überschreibbare Methode eine nicht anpassbare Argumentvalidierung erforderlich ist. Ganz anders herum:

Wenn eine Methode so entworfen werden soll, dass sie in allen Ableitungen der Klasse (oder allgemeiner gesagt in einem anpassbaren und einem nicht anpassbaren Teil) einige feste Argumentvalidierungen liefert, ist es sinnvoll, den Einstiegspunkt nicht virtuell zu machen Stellen Sie stattdessen eine virtuelle oder abstrakte Methode für den anpassbaren Teil bereit, der intern aufgerufen wird.

Es gibt jedoch viele Beispiele, bei denen es durchaus sinnvoll ist, eine öffentliche virtuelle Methode zu haben, da es keinen festen, nicht anpassbaren Teil gibt: Sehen Sie sich Standardmethoden wie ToStringoder Equalsoder an. GetHashCodeWürde es das Design der objectKlasse verbessern , wenn diese nicht öffentlich sind und gleichzeitig virtuell? Ich glaube nicht.

Oder in Bezug auf Ihren eigenen Code: Wenn der Code in der Basisklasse endgültig und absichtlich so aussieht

 public void Method1(string argument)
 {
    // nothing to validate here, all strings including null allowed
    this.Method1Core(argument);
 }

Diese Trennung zwischen Method1und Method1Corekompliziert die Dinge nur ohne ersichtlichen Grund.

Doc Brown
quelle
1
Bei der ToString()Methode hat Microsoft sie besser nicht-virtuell gemacht und eine virtuelle Vorlagenmethode eingeführt ToStringCore(). Warum: aus diesem Grund: ToString()- Hinweis an die Erben . Sie geben an, dass ToString()nicht null zurückgegeben werden soll. Sie hätten diese Forderung durch Umsetzung erzwingen können ToString() => ToStringCore() ?? string.Empty.
Peter Perot
9
@PeterPerot: Die Richtlinie, mit der Sie verlinkt sind, empfiehlt auch, nicht zurückzukehren. Haben string.EmptySie das bemerkt? Und es werden viele andere Dinge empfohlen, die nicht durch die Einführung einer ToStringCoreMethode im Code erzwungen werden können. Daher ist diese Technik wahrscheinlich nicht das richtige Werkzeug für ToString.
Doc Brown
3
@Theraot: Sicher kann man einige Gründe oder Argumente finden, warum ToString und Equals oder GetHashcode anders entworfen wurden, aber heute sind sie so wie sie sind (zumindest denke ich, dass ihr Design gut genug ist, um ein gutes Beispiel zu geben).
Doc Brown
1
@PeterPerot „Ich sah eine Menge Code wie anyObjectIGot.ToString()statt anyObjectIGot?.ToString()“ - wie dies relevant ist? Ihr ToStringCore()Ansatz würde verhindern, dass eine Nullzeichenfolge zurückgegeben wird, aber es würde trotzdem einen werfen, NullReferenceExceptionwenn das Objekt Null ist.
4.
1
@PeterPerot Ich habe nicht versucht, ein Argument der Behörde zu übermitteln. Es ist nicht so viel, das Microsoft verwendet public virtual, aber es gibt Fälle, in denen public virtuales in Ordnung ist. Wir könnten argumentieren, dass ein leerer, nicht anpassbarer Teil den Code zukunftssicher macht ... Aber das funktioniert nicht. Ein Zurückgehen und Ändern kann abgeleitete Typen beschädigen. Somit verdient es nichts.
Theraot
6

Wenn Sie dies so tun, wie es Ihr Kollege vorschlägt, wird der Implementierer der Basisklasse flexibler. Dies bringt jedoch auch eine größere Komplexität mit sich, die in der Regel nicht durch den vermuteten Nutzen gerechtfertigt ist.

Geist , dass die erhöhte Flexibilität für die Implementierer Basisklasse kommt auf Kosten von weniger Flexibilität für die zwingende Partei. Sie bekommen ein auferlegtes Verhalten, für das sie sich möglicherweise nicht besonders interessieren. Für sie sind die Dinge nur noch steifer geworden. Dies kann gerechtfertigt und hilfreich sein, hängt jedoch vom jeweiligen Szenario ab.

Die Namenskonvention, um dies zu implementieren (die ich kenne), besteht darin, den guten Namen für die öffentliche Schnittstelle zu reservieren und dem Namen der internen Methode das Präfix "Do" voranzustellen.

Ein nützlicher Fall ist, wenn für die ausgeführte Aktion einige Einstellungen erforderlich sind und einige beendet werden müssen. Öffnen Sie einen Stream und schließen Sie ihn, nachdem der Overrider abgeschlossen ist. Im Allgemeinen die gleiche Art der Initialisierung und Finalisierung. Es ist ein gültiges Muster, aber es wäre sinnlos, es in allen abstrakten und virtuellen Szenarien anzuwenden.

Martin Maat
quelle
1
Das Do- Methodenpräfix ist eine Option. Microsoft nutzt oft die Core - Methode Postfix.
Peter Perot
@ Peter Perot. Ich habe das Core-Präfix in keinem Microsoft-Material gesehen, aber dies kann daran liegen, dass ich in letzter Zeit nicht viel Aufmerksamkeit geschenkt habe. Ich vermute, sie haben kürzlich damit begonnen, nur um den Spitznamen Core bekannt zu machen und einen Namen für .NET Core zu machen.
Martin Maat
Nein, es ist ein alter Hut: BindingList. Außerdem habe ich die Empfehlung irgendwo gefunden, vielleicht deren Framework Design Guidelines oder ähnliches. Und: Es ist ein Postfix . ;-)
Peter Perot
Weniger Flexibilität für die abgeleitete Klasse ist der Punkt. Die Basisklasse ist eine Abstraktionsgrenze. Die Basisklasse teilt dem Verbraucher mit, was seine öffentliche API tut, und definiert, welche API erforderlich ist, um diese Ziele zu erreichen. Wenn eine abgeleitete Klasse eine öffentliche Methode in der Basisklasse überschreiben kann, besteht ein erhöhtes Risiko, dass das Liskov-Substitutionsprinzip verletzt wird.
Adrian McCarthy
2

In C ++ wird dies als nicht-virtuelles Schnittstellenmuster (NVI) bezeichnet. (Früher hieß es Template-Methode. Das war verwirrend, aber einige ältere Artikel haben diese Terminologie.) NVI wird von Herb Sutter beworben, der mindestens einige Male darüber geschrieben hat. Ich denke, einer der frühesten ist hier .

Wenn ich mich richtig erinnere, ist die Prämisse, dass eine abgeleitete Klasse nicht ändern sollte, was die Basisklasse tut, sondern wie sie es tut.

Beispielsweise verfügt eine Form möglicherweise über eine Move-Methode zum Verschieben der Form. Konkrete Implementierungen (z. B. Square und Circles) sollten Move nicht direkt außer Kraft setzen, da Shape definiert, was Moving bedeutet (auf konzeptioneller Ebene). In Bezug auf die interne Darstellung der Position kann ein Quadrat andere Implementierungsdetails aufweisen als ein Kreis. Sie müssen daher eine Methode außer Kraft setzen, um die Verschiebungsfunktion bereitzustellen.

In einfachen Beispielen läuft dies oft auf einen öffentlichen Umzug hinaus, bei dem die gesamte Arbeit an eine private virtuelle ReallyDoTheMove-Instanz delegiert wird.

Diese Eins-zu-Eins-Korrespondenz ist jedoch keine Voraussetzung. Beispielsweise können Sie der öffentlichen API von Shape eine Animate-Methode hinzufügen und diese implementieren, indem Sie ReallyDoTheMove in einer Schleife aufrufen. Am Ende stehen Ihnen zwei öffentliche, nicht virtuelle Methoden-APIs zur Verfügung, die beide auf der einen privaten abstrakten Methode basieren. Ihre Kreise und Quadrate müssen keine zusätzliche Arbeit leisten, und Animate kann nicht außer Kraft gesetzt werden .

Die Basisklasse definiert die öffentliche Schnittstelle, die von Verbrauchern verwendet wird, und definiert eine Schnittstelle von primitiven Operationen, die zum Implementieren dieser öffentlichen Methoden erforderlich sind. Die abgeleiteten Typen sind dafür verantwortlich, Implementierungen dieser primitiven Operationen bereitzustellen.

Ich kenne keine Unterschiede zwischen C # und C ++, die diesen Aspekt des Klassendesigns verändern würden.

Adrian McCarthy
quelle
Guter Fund! Jetzt erinnere ich mich, dass ich genau diesen Post gefunden habe, auf den Ihr zweiter Link (oder eine Kopie davon) in den 2000er Jahren verweist. Ich erinnere mich, dass ich nach mehr Beweisen für die Behauptung meines Kollegen gesucht habe und im C # -Kontext nichts gefunden habe, außer C ++. Dies. Ist. Es! :-) Aber zurück im C # -Land scheint dieses Muster nicht viel verwendet zu werden. Vielleicht wurde den Leuten klar, dass das spätere Hinzufügen von Basisfunktionalität auch abgeleitete Klassen beschädigen kann und dass die strikte Verwendung von TMP oder NVIP anstelle von öffentlichen virtuellen Methoden nicht immer Sinn macht.
Peter Perot
Hinweis für sich selbst: Gut zu wissen, dass dieses Muster einen Namen hat: NVIP.
Peter Perot