Nigdy nie udostępniaj publicznie elementów wirtualnych / abstrakcyjnych - naprawdę?

20

W 2000 roku mój kolega powiedział mi, że to anty-wzorzec, aby publiczne metody były wirtualne lub abstrakcyjne.

Na przykład uważał taką klasę za niezbyt dobrze zaprojektowaną:

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

Stwierdził to

  • twórca klasy pochodnej, która implementuje Method1i zastępuje, Method2musi powtórzyć sprawdzanie poprawności argumentów.
  • na wypadek, gdyby twórca klasy podstawowej zdecydował się dodać coś wokół dostosowywalnej części Method1lub Method2później, nie może tego zrobić.

Zamiast tego mój kolega zaproponował takie podejście:

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

Powiedział mi, że upublicznianie publicznych metod (lub właściwości) jest tak samo złe, jak upublicznianie pól. Dzięki zawijaniu pól we właściwości można w razie potrzeby przechwycić później dostęp do tych pól. To samo dotyczy publicznych wirtualnych / abstrakcyjnych członków: zawijanie ich w sposób pokazany w ProtectedAbstractOrVirtualklasie pozwala programistom klasy podstawowej przechwytywać wszelkie wywołania, które przechodzą do metod wirtualnych / abstrakcyjnych.

Ale nie uważam tego za wytyczne projektowe. Nawet Microsoft tego nie przestrzega: wystarczy spojrzeć na Streamklasę, aby to sprawdzić.

Co sądzisz o tej wytycznej? Czy to ma sens, czy uważasz, że to komplikuje API?

Peter Perot
źródło
5
Metody tworzenia virtual pozwalają na opcjonalne zastąpienie. Twoja metoda powinna być prawdopodobnie publiczna, ponieważ może nie zostać zastąpiona. Tworzenie metod abstract zmusza do ich przesłonięcia; prawdopodobnie powinny protected, ponieważ nie są szczególnie przydatne w publickontekście.
Robert Harvey
4
W rzeczywistości protectedjest najbardziej przydatny, gdy chcesz narazić prywatnych członków klasy abstrakcyjnej na klasy pochodne. W każdym razie nie martwię się szczególnie o opinię twojego przyjaciela; wybierz modyfikator dostępu, który najlepiej pasuje do konkretnej sytuacji.
Robert Harvey
4
Twój kolega opowiadał się za wzorcem metody szablonu . Mogą istnieć przypadki użycia dla obu sposobów, w zależności od wzajemnych zależności między tymi dwiema metodami.
Greg Burghardt
8
@GregBurghardt: brzmi jak kolega z PO sugeruje, aby zawsze używać wzorca metody szablonu, niezależnie od tego, czy jest to wymagane, czy nie. To typowe nadużywanie wzorów - jeśli ktoś ma młotek, prędzej czy później każdy problem zaczyna wyglądać jak gwóźdź ;-)
Doc Brown
3
@PeterPerot: Nigdy nie miałem problemu z uruchomieniem prostych DTO z tylko polami publicznymi, a kiedy takie DTO wymagało członków z logiką biznesową, przekieruj ich do klas o właściwościach. Z pewnością jest inaczej, gdy ktoś pracuje jako dostawca biblioteki i musi dbać o to, by nie zmieniać publicznych interfejsów API, a nawet przekształcenie pola publicznego w właściwość publiczną o tej samej nazwie może powodować problemy.
Doc Brown

Odpowiedzi:

30

Powiedzenie

że jest to anty-wzorzec, aby metody publiczne były wirtualne lub abstrakcyjne, ponieważ twórca klasy pochodnej, który implementuje Metodę 1 i zastępuje Metodę 2, musi powtórzyć walidację argumentu

miesza przyczynę i skutek. Zakłada się, że każda nadpisywana metoda wymaga niepoprawnego sprawdzania poprawności argumentów. Ale jest na odwrót:

Jeśli ktoś chce zaprojektować metodę w taki sposób, aby zapewniała pewne stałe sprawdzanie poprawności argumentów we wszystkich pochodnych klasy (lub - bardziej ogólnie - w części konfigurowalnej i niemożliwej do dostosowania), wówczas sensowne jest, aby punkt wejścia był nie-wirtualny , a zamiast tego zapewnia wirtualną lub abstrakcyjną metodę dla dostosowywanej części, która jest nazywana wewnętrznie.

Ale istnieje wiele przykładów, gdzie to ma sens mieć publiczną wirtualną metodę, ponieważ nie jest stały zakaz konfigurowalny część: spojrzenie na standardowych metod, takich jak ToStringlub Equalsczy GetHashCode- by poprawić konstrukcję objectklasy mają te nie publiczne i wirtualny w tym samym czasie? Nie wydaje mi się

Lub, jeśli chodzi o własny kod: kiedy kod w klasie bazowej wreszcie i celowo wygląda tak

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

takie oddzielenie Method1i Method1Corekomplikuje tylko rzeczy bez wyraźnego powodu.

Doktor Brown
źródło
1
W przypadku ToString()metody Microsoft lepiej uczynił ją nie-wirtualną i wprowadził metodę wirtualnego szablonu ToStringCore(). Dlaczego: z tego powodu: ToString()- Uwaga dla spadkobierców . Oświadczają, że ToString()nie powinny zwracać wartości null. Mogliby sprostać temu żądaniu poprzez wdrożenie ToString() => ToStringCore() ?? string.Empty.
Peter Perot,
9
@PeterPerot: wytyczne, do których się odnosisz, również zalecają, aby nie wracać string.Empty, zauważyłeś? I zaleca wiele innych rzeczy, których nie można wymusić w kodzie przez wprowadzenie czegoś takiego jak ToStringCoremetoda. Ta technika prawdopodobnie nie jest odpowiednim narzędziem ToString.
Doc Brown
3
@Theraot: z pewnością można znaleźć kilka powodów lub argumentów, dlaczego ToString i Equals lub GetHashcode mogły zostać zaprojektowane inaczej, ale dziś są takie, jakie są (przynajmniej uważam, że ich konstrukcja jest wystarczająca, aby stanowić dobry przykład).
Doc Brown
1
@PeterPerot „Widziałem dużo kodu podobnego do anyObjectIGot.ToString()zamiast anyObjectIGot?.ToString()” - jak to ma znaczenie? Twoje ToStringCore()podejście zapobiegnie zwróceniu łańcucha zerowego, ale nadal spowoduje wyrzucenie znaku, NullReferenceExceptionjeśli obiekt ma wartość zerową.
IMil
1
@PeterPerot Nie próbowałem przekazać argumentu od władzy. Nie tyle korzysta Microsoft public virtual, ale są przypadki, w których public virtualjest w porządku. Moglibyśmy argumentować za pustą częścią, której nie można dostosowywać, co czyni kod kodem na przyszłość ... Ale to nie działa. Cofanie się i zmienianie może spowodować uszkodzenie typów pochodnych. W ten sposób nic nie zarabia.
Theraot
6

Robienie tego w sposób sugerowany przez kolegę zapewnia większą elastyczność implementatorowi klasy podstawowej. Ale z tym wiąże się także większa złożoność, która zwykle nie jest uzasadniona domniemanymi korzyściami.

Pamiętaj, że większa elastyczność dla implementatora klasy podstawowej odbywa się kosztem mniejszej elastyczności dla strony nadrzędnej. Otrzymują pewne narzucone zachowania, o które nie mogą szczególnie dbać. Dla nich sprawy stały się bardziej sztywne. Może to być uzasadnione i pomocne, ale wszystko zależy od scenariusza.

Konwencja nazewnictwa w celu implementacji tego (o czym wiem) polega na zarezerwowaniu dobrej nazwy interfejsu publicznego i poprzedzeniu nazwy metody wewnętrznej słowem „Do”.

Jednym z użytecznych przypadków jest sytuacja, w której wykonywana czynność wymaga konfiguracji i zamknięcia. Jak otworzyć strumień i zamknąć go po zakończeniu overrider. Ogólnie ten sam rodzaj inicjalizacji i finalizacji. Jest to prawidłowy wzorzec do użycia, ale nie ma sensu narzucać jego zastosowania we wszystkich abstrakcyjnych i wirtualnych scenariuszach.

Martin Maat
źródło
1
Do metoda prefiks jest jedna opcja. Microsoft często korzysta z postfiksu metody Core .
Peter Perot,
@Peter Perot. Nigdy nie widziałem przedrostka Core w żadnym materiale Microsoft, ale może to być spowodowane tym, że ostatnio nie zwracałem zbytniej uwagi. Podejrzewam, że zaczęli to robić niedawno, aby promować moniker Core i stworzyć nazwę dla .NET Core.
Martin Maat
Nie, to stary kapelusz: BindingList. Ponadto znalazłem gdzieś zalecenie, może ich Wytyczne dotyczące projektowania ram lub coś podobnego. I: to postfiks . ;-)
Peter Perot
Chodzi o mniejszą elastyczność dla klasy pochodnej. Klasa podstawowa stanowi granicę abstrakcji. Klasa podstawowa mówi konsumentowi, co robi jego publiczny interfejs API, i określa, że ​​interfejs API jest wymagany do osiągnięcia tych celów. Jeśli klasa pochodna może zastąpić metodę publiczną w klasie podstawowej, istnieje zwiększone ryzyko naruszenia zasady substytucji Liskowa.
Adrian McCarthy
2

W języku C ++ jest to nazywane wzorcem interfejsu innego niż wirtualny (NVI). (Kiedyś nazywało się to Metodą Szablonu. To było mylące, ale niektóre starsze artykuły mają taką terminologię.) NVI promuje Herb Sutter, który napisał o tym przynajmniej kilka razy. Myślę, że jeden z najwcześniej jest tutaj .

Jeśli dobrze pamiętam, założeniem jest, że klasa pochodna nie powinna zmieniać tego, co robi klasa podstawowa, ale jak to robi.

Na przykład Kształt może mieć metodę Przenieś w celu przeniesienia kształtu. Konkretne implementacje (np. Kwadrat i koła) nie powinny bezpośrednio zastępować ruchu, ponieważ Shape określa, co oznacza ruch (na poziomie koncepcyjnym). Kwadrat może mieć szczegóły implementacji różnicy niż Okrąg pod względem sposobu wewnętrznej reprezentacji pozycji, więc będą musieli zastąpić jakąś metodę, aby zapewnić funkcjonalność Przenieś.

W prostych przykładach często sprowadza się to do publicznego Move, który po prostu deleguje całą pracę do prywatnego wirtualnego ReallyDoTheMove, więc wydaje się, że to dużo narzutu bez korzyści.

Ale ta korespondencja jeden do jednego nie jest wymagana. Na przykład możesz dodać metodę Animate do publicznego interfejsu API Shape, który może ją zaimplementować, wywołując ReallyDoTheMove w pętli. W efekcie powstają dwa interfejsy API publicznych metod innych niż wirtualne, które opierają się na jednej prywatnej metodzie abstrakcyjnej. Twoje kręgi i kwadraty nie muszą wykonywać żadnej dodatkowej pracy, podobnie jak Animate .

Klasa podstawowa definiuje interfejs publiczny używany przez konsumentów i definiuje interfejs operacji prymitywnych, których potrzebuje do zaimplementowania tych metod publicznych. Typy pochodne są odpowiedzialne za zapewnienie implementacji tych prymitywnych operacji.

Nie znam żadnych różnic między C # i C ++, które mogłyby zmienić ten aspekt projektowania klas.

Adrian McCarthy
źródło
Dobre znalezisko! Teraz pamiętam, że znalazłem dokładnie ten post, na który wskazuje twój drugi link (lub jego kopia) w 2000 roku. Pamiętam, że szukałem więcej dowodów na roszczenie mojego kolegi i że nie znalazłem niczego w kontekście C #, ale C ++. To. Jest. To! :-) Ale w C # wydaje się, że ten wzór nie jest używany zbyt często. Być może ludzie zdali sobie sprawę, że późniejsze dodanie podstawowej funkcjonalności może również przełamać klasy pochodne i że ścisłe stosowanie TMP lub NVIP zamiast publicznych metod wirtualnych nie zawsze ma sens.
Peter Perot,
Uwaga do siebie: dobrze wiedzieć, że ten wzór ma nazwę: NVIP.
Peter Perot,