Czy tworzenie podklas dla określonych instancji jest złą praktyką?

37

Rozważ następujący projekt

public class Person
{
    public virtual string Name { get; }

    public Person (string name)
    {
        this.Name = name;
    }
}

public class Karl : Person
{
    public override string Name
    {
        get
        {
            return "Karl";
        }
    }
}

public class John : Person
{
    public override string Name
    {
        get
        {
            return "John";
        }
    }
}

Myślisz, że coś tu jest nie tak? Dla mnie zajęcia Karla i Johna powinny być tylko instancjami zamiast zajęć, ponieważ są dokładnie takie same jak:

Person karl = new Person("Karl");
Person john = new Person("John");

Dlaczego miałbym tworzyć nowe klasy, gdy instancje są wystarczające? Klasy nic nie dodają do instancji.

Ignacio Soler Garcia
źródło
17
Niestety znajdziesz to w wielu kodach produkcyjnych. Posprzątaj, kiedy możesz.
Adam Zuckerman
14
To świetny projekt - jeśli programista chce uczynić się niezbędnym dla każdej zmiany danych biznesowych i nie ma problemów z dostępnością 24/7 ;-)
Doc Brown
1
Oto rodzaj pokrewnego pytania, w którym OP wydaje się wierzyć w przeciwieństwo konwencjonalnej mądrości. programmers.stackexchange.com/questions/253612/…
Dunk
11
Projektuj swoje klasy w zależności od zachowania, a nie danych. Według mnie podklasy nie dodają żadnego nowego zachowania do hierarchii.
Songo
2
Jest to powszechnie stosowane z anonimowymi podklasami (takimi jak programy obsługi zdarzeń) w Javie, zanim lambdas pojawiło się w Javie 8 (która jest taka sama z inną składnią).
marczellm

Odpowiedzi:

77

Nie ma potrzeby posiadania określonych podklas dla każdej osoby.

Masz rację, powinny to być instancje.

Cel podklas: rozszerzenie klas nadrzędnych

Podklasy służą do rozszerzania funkcjonalności zapewnianej przez klasę nadrzędną. Na przykład możesz mieć:

  • Klasa nadrzędna, Batteryktóra może mieć Power()coś i może mieć Voltagewłaściwość,

  • I podklasę RechargeableBattery, która może dziedziczyć Power()i Voltage, ale może również być Recharge()d.

Zauważ, że możesz przekazać instancję RechargeableBatteryklasy jako parametr do dowolnej metody, która przyjmuje Batteryargument jako argument. Nazywa się to zasadą podstawienia Liskowa , jedną z pięciu zasad SOLID. Podobnie w rzeczywistości, jeśli mój odtwarzacz MP3 akceptuje dwie baterie AA, mogę je zastąpić dwiema bateriami AA.

Zauważ, że czasami trudno jest ustalić, czy musisz użyć pola, czy podklasy, aby przedstawić różnicę między czymś. Na przykład, jeśli musisz obsługiwać akumulatory AA, AAA i 9-woltowe, czy stworzyłbyś trzy podklasy lub użyłby wyliczenia? „Zamień podklasę na pola” w Refaktoryzacji autorstwa Martina Fowlera, strona 232, może dać ci kilka pomysłów i sposobu przejścia od jednego do drugiego.

W twoim przykładzie Karli Johnnie rozszerzaj niczego, ani nie zapewniają one żadnej dodatkowej wartości: możesz mieć dokładnie taką samą funkcjonalność, używając Personbezpośrednio klasy. Posiadanie większej liczby wierszy kodu bez dodatkowej wartości nigdy nie jest dobre.

Przykład uzasadnienia biznesowego

Co może być uzasadnieniem biznesowym, w którym sensowne byłoby utworzenie podklasy dla konkretnej osoby?

Załóżmy, że tworzymy aplikację, która zarządza osobami pracującymi w firmie. Aplikacja zarządza również uprawnieniami, więc Helen, księgowa, nie może uzyskać dostępu do repozytorium SVN, ale Thomas i Mary, dwaj programiści, nie mogą uzyskać dostępu do dokumentów związanych z rachunkowością.

Jimmy, wielki szef (założyciel i dyrektor generalny firmy) ma bardzo szczególne uprawnienia, których nikt nie ma. Może na przykład zamknąć cały system lub zwolnić osobę. Masz pomysł.

Najgorszym modelem dla takiej aplikacji jest posiadanie klas takich jak:

          Diagram klas pokazuje klasy Helen, Thomas, Mary i Jimmy i ich wspólnego rodzica: abstrakcyjną klasę Person.

ponieważ duplikacja kodu powstanie bardzo szybko. Nawet w bardzo podstawowym przykładzie czterech pracowników zduplikujesz kod między klasami Thomas i Mary. To popchnie cię do stworzenia wspólnej klasy nadrzędnej Programmer. Ponieważ możesz mieć także wielu księgowych, prawdopodobnie również utworzysz Accountantzajęcia.

          Diagram klas pokazuje abstrakcyjną osobę z trójką dzieci: abstrakcyjną księgową, abstrakcyjnego programistę i Jimmy'ego.  Księgowa ma podklasę Helen, a programista ma dwie podklasy: Thomas i Mary.

Teraz zauważasz, że posiadanie klasy Helennie jest zbyt przydatne, a także przechowywanie Thomasi Mary: większość kodu działa w każdym razie na wyższym poziomie - na poziomie księgowych, programistów i Jimmy'ego. Serwer SVN nie dba o to, czy to Thomas czy Mary potrzebują dostępu do dziennika - musi tylko wiedzieć, czy jest programistą czy księgowym.

if (person is Programmer)
{
    this.AccessGranted = true;
}

W rezultacie usuwasz klasy, których nie używasz:

          Diagram klas zawiera podklasy Księgowego, Programisty i Jimmy'ego wraz ze wspólną klasą nadrzędną: abstrakcyjna Osoba.

„Ale mogę utrzymać Jimmy'ego w obecnej postaci, ponieważ zawsze byłby tylko jeden dyrektor generalny, jeden wielki szef - Jimmy”. Co więcej, Jimmy jest często używany w twoim kodzie, który faktycznie wygląda bardziej tak, a nie jak w poprzednim przykładzie:

if (person is Jimmy)
{
    this.GiveUnrestrictedAccess(); // Because Jimmy should do whatever he wants.
}
else if (person is Programmer)
{
    this.AccessGranted = true;
}

Problem z tym podejściem polega na tym, że Jimmy wciąż może zostać potrącony przez autobus, i byłby nowy dyrektor generalny. Albo zarząd może zdecydować, że Mary jest tak wielka, że ​​powinna być nowym dyrektorem generalnym, a Jimmy zostałby zdegradowany do stanowiska sprzedawcy, więc teraz musisz przejść cały kod i wszystko zmienić.

Arseni Mourzenko
źródło
6
@DocBrown: Często jestem zaskoczony widząc, że krótkie odpowiedzi, które nie są złe, ale niewystarczająco głębokie, mają znacznie wyższy wynik niż odpowiedzi wyjaśniające dogłębnie temat. Prawdopodobnie zbyt wiele osób nie lubi dużo czytać, więc bardzo krótkie odpowiedzi wyglądają dla nich bardziej atrakcyjnie. Mimo to zredagowałem swoją odpowiedź, aby podać przynajmniej jakieś wyjaśnienie.
Arseni Mourzenko
3
Krótkie odpowiedzi są zwykle publikowane jako pierwsze. Wcześniejsze posty zdobywają więcej głosów.
Tim B
1
@ TimB: Zwykle zaczynam od średnich odpowiedzi, a następnie edytuję je, aby były bardzo kompletne (oto przykład , biorąc pod uwagę, że prawdopodobnie było około piętnaście wersji, tylko cztery). Zauważyłem, że im dłużej z czasem staje się odpowiedzią, tym mniej przychylnych głosów otrzymuje; podobne pytanie, które pozostaje krótkie, przyciąga więcej entuzjastów.
Arseni Mourzenko
Zgadzam się z @DocBrown. Na przykład dobra odpowiedź brzmiałaby „podklasa zachowania , a nie treści”, i odsyłam do pewnych odniesień, których nie będę robił w tym komentarzu.
user949300,
2
W przypadku, gdy nie było jasne z ostatniego akapitu: Nawet jeśli masz tylko 1 osobę z nieograniczonym dostępem, nadal powinieneś ustawić tę osobę jako instancję oddzielnej klasy, która implementuje nieograniczony dostęp. Sprawdzanie, czy dana osoba sama powoduje interakcję kod-dane, może otworzyć całą puszkę robaków. Na przykład, co jeśli Rada Dyrektorów podejmie decyzję o powołaniu triumwiratu na stanowisko CEO? Gdybyś nie miał klasy „Nieograniczony”, nie musiałbyś po prostu aktualizować obecnego CEO, ale także dodać jeszcze 2 kontrole dla pozostałych członków. Jeśli tak, ustaw je jako „Nieograniczone”.
Nzall,
15

Głupio jest używać tego rodzaju struktury klas tylko po to, by zmieniać szczegóły, które oczywiście powinny być ustawialnymi polami w instancji. Ale to jest szczególne w twoim przykładzie.

Tworzenie różnych Personklas jest prawie na pewno złym pomysłem - musiałbyś zmienić swój program i dokonać ponownej kompilacji za każdym razem, gdy nowa Osoba wchodzi do Twojej domeny. Nie oznacza to jednak, że dziedziczenie z istniejącej klasy, tak że gdzieś zwracany jest inny ciąg, nie jest czasem przydatne.

Różnica polega na tym, że oczekuje się, że byty reprezentowane przez tę klasę będą się różnić. Zakłada się, że ludzie prawie zawsze przychodzą i odchodzą, i oczekuje się, że prawie każda poważna aplikacja zajmująca się użytkownikami będzie mogła dodawać i usuwać użytkowników w czasie wykonywania. Ale jeśli modelujesz takie rzeczy, jak różne algorytmy szyfrowania, obsługa nowej i tak jest prawdopodobnie poważną zmianą i sensowne jest wynalezienie nowej klasy, której myName()metoda zwraca inny ciąg (i której perform()metoda prawdopodobnie robi coś innego).

Kilian Foth
źródło
12

Dlaczego miałbym tworzyć nowe klasy, gdy instancje są wystarczające?

W większości przypadków nie. Twój przykład jest naprawdę dobrym przykładem, w którym to zachowanie nie wnosi rzeczywistej wartości.

Narusza to również zasadę Otwartego Zamknięcia , ponieważ podklasy zasadniczo nie są rozszerzeniem, lecz modyfikują wewnętrzne funkcjonowanie rodzica. Co więcej, publiczny konstruktor rodzica jest teraz irytujący w podklasach, a zatem interfejs API stał się mniej zrozumiały.

Jednak czasami, jeśli kiedykolwiek będziesz mieć tylko jedną lub dwie specjalne konfiguracje, które są często używane w całym kodzie, czasem jest wygodniej i mniej czasochłonnie po prostu podklasować rodzica, który ma złożony konstruktor. W takich szczególnych przypadkach nie widzę nic złego w takim podejściu. Nazwijmy to konstruktorem curry j / k

Sokół
źródło
Nie jestem pewien, czy zgadzam się z akapitem OCP. Jeśli klasa wystawia ustawiającego właściwości swoim potomkom, to przy założeniu, że postępuje zgodnie z zasadami dobrego projektowania, właściwość nie powinna być częścią jego wewnętrznego działania
Ben Aaronson
@BenAaronson Dopóki zachowanie samej nieruchomości nie zostanie zmienione, nie naruszasz OCP, ale tutaj tak się dzieje. Oczywiście możesz użyć tej właściwości w jej wewnętrznych działaniach. Ale nie powinieneś zmieniać istniejącego zachowania! Powinieneś tylko przedłużać zachowanie, a nie zmieniać go przez dziedziczenie. Oczywiście są wyjątki od każdej reguły.
Falcon
1
Czy jakiekolwiek wykorzystanie członków wirtualnych stanowi naruszenie OCP?
Ben Aaronson
3
@ Falcon Nie ma potrzeby uzyskiwania nowej klasy, aby uniknąć powtarzających się wywołań złożonego konstruktora. Wystarczy utworzyć fabryki dla typowych przypadków i sparametryzować małe odmiany.
Keen
@BenAaronson Powiedziałbym, że posiadanie wirtualnych członków, którzy mają faktyczną implementację, jest takim naruszeniem. Członkowie abstrakcyjni lub powiedzmy metody, które nic nie robią, byłyby ważnymi punktami rozszerzenia. Zasadniczo, gdy spojrzysz na kod klasy bazowej, wiesz, że będzie ona działać bez zmian, zamiast że każda nie-ostateczna metoda będzie kandydatem do zastąpienia czymś zupełnie innym. Innymi słowy: sposoby, w jakie klasa dziedzicząca może wpływać na zachowanie klasy podstawowej, będą jawne i ograniczone do „addytywności”.
millimoose
8

Jeśli taki jest zakres praktyki, to zgadzam się, że jest to zła praktyka.

Jeśli John i Karl zachowują się inaczej, sytuacja się nieco zmienia. Może być tak, że personma metodę, w cleanRoom(Room room)której okazuje się, że John jest świetnym współlokatorem i bardzo skutecznie sprząta, ale Karl nie jest i nie bardzo czyści pokój.

W takim przypadku sensowne byłoby, aby były one własną podklasą ze zdefiniowanym zachowaniem. Są lepsze sposoby na osiągnięcie tego samego (na przykład CleaningBehaviorklasy), ale przynajmniej w ten sposób nie jest straszne naruszenie zasad OO.

corsiKa
źródło
Żadnych innych zachowań, tylko różne dane. Dzięki.
Ignacio Soler Garcia
6

W niektórych przypadkach wiesz, że będzie tylko określona liczba instancji, a wtedy użycie tego podejścia byłoby w porządku (choć moim zdaniem brzydkie). Lepiej jest użyć wyliczenia, jeśli pozwala na to język.

Przykład Java:

public enum Person
{
    KARL("Karl"),
    JOHN("John")

    private final String name;

    private Person(String name)
    {
        this.name = name;
    }

    public String getName()
    {
        return this.name;
    }
}
Adam
źródło
6

Wiele osób już odpowiedziało. Myślałem, że dam własną perspektywę.


Dawno, dawno temu pracowałem nad aplikacją (i nadal tak robię), która tworzy muzykę.

Aplikacja miała abstrakcyjną Scaleklasę z kilku podklas: CMajor, DMinoritp Scalewyglądał mniej więcej tak więc:

public abstract class Scale {
    protected Note[] notes;
    public Scale() {
        loadNotes();
    }
    // .. some other stuff ommited
    protected abstract void loadNotes(); /* subclasses put notes in the array
                                          in this method. */
}

Generatory muzyki współpracowały z konkretną Scaleinstancją w celu generowania muzyki. Użytkownik wybiera skalę z listy, z której ma być generowana muzyka.

Pewnego dnia przyszedł mi do głowy fajny pomysł: dlaczego nie pozwolić użytkownikowi na tworzenie własnych wag? Użytkownik wybiera notatki z listy, naciska przycisk, a nowa skala zostanie dodana do listy dostępnych skal.

Ale nie byłem w stanie tego zrobić. Stało się tak, ponieważ wszystkie skale są już ustawione w czasie kompilacji - ponieważ są wyrażone jako klasy. Potem uderzyło mnie to:

Często intuicyjne jest myślenie w kategoriach „nadklas i podklas”. Prawie wszystko można wyrazić za pomocą tego systemu: nadklasa Personi podklasy Johnoraz Mary; nadklasa Cari podklasy Volvooraz Mazda; nadklasa Missilei podklasy SpeedRocked, LandMineoraz TrippleExplodingThingy.

Myślenie w ten sposób jest bardzo naturalne, szczególnie dla osoby stosunkowo nowej w OO.

Ale zawsze powinniśmy pamiętać, że klasy to szablony , a obiekty to zawartość wlewana do tych szablonów . Możesz wlać dowolną zawartość do szablonu, tworząc niezliczone możliwości.

Wypełnienie szablonu nie jest zadaniem podklasy. To zadanie obiektu. Zadaniem podklasy jest dodanie faktycznej funkcjonalności lub rozwinięcie szablonu .

I dlatego powinienem był stworzyć konkretną Scaleklasę z Note[]polem i pozwolić obiektom wypełnić ten szablon ; ewentualnie przez konstruktora lub coś takiego. I w końcu tak zrobiłem.

Za każdym razem, gdy projektujesz szablon w klasie (np. Pusty Note[]element, który należy wypełnić lub String namepole, któremu należy przypisać wartość), pamiętaj, że wypełnianie szablonu jest zadaniem obiektów tej klasy ( lub ewentualnie osoby tworzące te obiekty). Podklasy mają na celu zwiększenie funkcjonalności, a nie wypełnianie szablonów.


Możesz mieć ochotę stworzyć „nadklasę Person, podklasy Johni Mary” system, tak jak to zrobiłeś, ponieważ podoba ci się formalność, jaką to daje.

W ten sposób możesz po prostu powiedzieć Person p = new Mary()zamiast Person p = new Person("Mary", 57, Sex.FEMALE). Sprawia, że ​​rzeczy są bardziej zorganizowane i bardziej uporządkowane. Ale, jak powiedzieliśmy, tworzenie nowej klasy dla każdej kombinacji danych nie jest dobrym podejściem, ponieważ przesadza z kodem za darmo i ogranicza cię pod względem możliwości działania.

Oto rozwiązanie: użyj podstawowej fabryki, może nawet być statyczna. Tak jak:

public final class PersonFactory {
    private PersonFactory() { }

    public static Person createJohn(){
        return new Person("John", 40, Sex.MALE);
    }

    public static Person createMary(){
        return new Person("Mary", 57, Sex.FEMALE);
    }

    // ...
}

W ten sposób możesz łatwo korzystać z „ustawień wstępnych” „dostarczanych z programem”, takich jak:, Person mary = PersonFactory.createMary()ale zastrzegasz sobie również prawo do dynamicznego projektowania nowych osób, na przykład w przypadku, gdy chcesz pozwolić użytkownikowi to zrobić . Na przykład:

// .. requesting the user for input ..
String name = // user input
int age = // user input
Sex sex = // user input, interpreted
Person newPerson = new Person(name, age, sex);

Lub jeszcze lepiej: zrób coś takiego:

public final class PersonFactory {
    private PersonFactory() { }

    private static Map<String, Person> persons = new HashMap<>();
    private static Map<String, PersonData> personBlueprints = new HashMap<>();

    public static void addPerson(Person person){
        persons.put(person.getName(), person);
    }

    public static Person getPerson(String name){
        return persons.get(name);
    }

    public static Person createPerson(String blueprintName){
        PersonData data = personBlueprints.get(blueprintName);
        return new Person(data.name, data.age, data.sex);
    }

    // .. or, alternative to the last method
    public static Person createPerson(String personName){
        Person blueprint = persons.get(personName);
        return new Person(blueprint.getName(), blueprint.getAge(), blueprint.getSex());
    }

}

public class PersonData {
    public String name;
    public int age;
    public Sex sex;

    public PersonData(String name, int age, Sex sex){
        this.name = name;
        this.age = age;
        this.sex = sex;
    }
}

Poniosło mnie. Myślę, że masz pomysł.


Podklasy nie mają na celu wypełnienia szablonów ustawionych przez ich nadklasy. Podklasy mają na celu dodanie funkcjonalności . Obiekty mają wypełniać szablony, po to są.

Nie powinieneś tworzyć nowej klasy dla każdej możliwej kombinacji danych. (Tak jak nie powinienem był tworzyć nowej Scalepodklasy dla każdej możliwej kombinacji Notes).

Jest wytyczną: za każdym razem, gdy tworzysz nową podklasę, zastanów się, czy dodaje ona nową funkcjonalność, która nie istnieje w tej podklasie. Jeśli odpowiedź na to pytanie brzmi „nie”, być może próbujesz „wypełnić szablon” nadklasy, w takim przypadku po prostu stwórz obiekt. (I ewentualnie Fabryka z „ustawieniami wstępnymi”, aby ułatwić życie).

Mam nadzieję, że to pomaga.

Aviv Cohn
źródło
To doskonały przykład, dziękuję bardzo.
Ignacio Soler Garcia
@ SoMoS Cieszę się, że mogłem pomóc.
Aviv Cohn
2

Jest to wyjątek od „tutaj powinny być instancje” - choć nieco ekstremalny.

Jeśli chcesz, aby kompilator wymuszał uprawnienia dostępu do metod na podstawie tego, czy jest to Karl czy John (w tym przykładzie), zamiast pytać implementację metody o sprawdzenie, która instancja została przekazana, możesz użyć różnych klas. Wymuszając różne klasy zamiast tworzenia instancji, umożliwia się sprawdzanie różnic między „Karl” i „John” w czasie kompilacji, a nie w czasie wykonywania, a może to być przydatne - szczególnie w kontekście bezpieczeństwa, w którym kompilatorowi można zaufać bardziej niż w czasie wykonywania kontrole kodu (na przykład) bibliotek zewnętrznych.

David Scholefield
źródło
2

Powielanie kodu jest uważane za złą praktykę .

Dzieje się tak przynajmniej częściowo, ponieważ linie kodów, a zatem rozmiar bazy kodów, koreluje z kosztami utrzymania i wadami .

Oto odpowiednia logika zdrowego rozsądku w tym konkretnym temacie:

1) Gdybym musiał to zmienić, ile miejsc musiałbym odwiedzić? Tutaj mniej znaczy lepiej.

2) Czy istnieje powód, dla którego odbywa się to w ten sposób? W twoim przykładzie - nie mogło być - ale w niektórych przykładach może istnieć jakiś powód, aby to zrobić. Jedno, co mogę sobie wyobrazić z góry głowy, to niektóre frameworki, takie jak wczesne Grails, w których dziedziczenie niekoniecznie działało ładnie z niektórymi wtyczkami do GORM. Bardzo rzadko.

3) Czy to jest czystsze i łatwiejsze do zrozumienia w ten sposób? Znowu nie ma tego w twoim przykładzie - ale istnieją pewne wzorce dziedziczenia, które mogą być bardziej rozciągliwe, w których oddzielne klasy są w rzeczywistości łatwiejsze do utrzymania (przychodzą na myśl pewne zastosowania wzorców poleceń lub strategii).

dcgregorya
źródło
1
czy to źle, czy nie, nie jest osadzone w kamieniu. Istnieją na przykład scenariusze, w których dodatkowe koszty tworzenia obiektów i wywołań metod mogą być tak niekorzystne dla wydajności, że aplikacja nie spełnia już specyfikacji.
jwenting
Punkt 2. Na pewno istnieją powody, dla których czasami. Dlatego musisz zapytać przed wypróżnieniem czyjegoś kodu.
dcgregorya
0

Istnieje przypadek użycia: utworzenie odwołania do funkcji lub metody jako zwykłego obiektu.

Widać to bardzo często w kodzie Java GUI:

ActionListener a = new ActionListener() {
    public void actionPerformed(ActionEvent arg0) {
        /* ... */
    }
};

Kompilator pomaga tutaj, tworząc nową anonimową podklasę.

Simon Richter
źródło