Zmniejszenie złożoności klasy

10

Patrzyłem na niektóre odpowiedzi i szukałem w Google, ale nie mogłem znaleźć nic pomocnego (tzn. Nie miałoby to niewygodnych skutków ubocznych).

Moim abstrakcyjnym problemem jest to, że mam obiekt i muszę wykonać na nim długą sekwencję operacji; Myślę o tym jak o rodzaju linii montażowej, jak budowanie samochodu.

Wierzę, że te obiekty nazywałyby się obiektami metod .

Więc w tym przykładzie w pewnym momencie miałbym CarWithoutUpholstery, na którym musiałbym uruchomić installBackSeat, installFrontSeat, installWoodenInserts (operacje nie kolidują ze sobą, a nawet mogą być wykonywane równolegle). Operacje te są wykonywane przez CarWithoutUpholstery.worker () i dają nowy obiekt, który byłby CarWithUpholstery, na którym następnie uruchomiłbym może cleanInsides (), VerifyNoUpholsteryDefects () i tak dalej.

Operacje w jednej fazie są już niezależne, tj. Już zmagam się z ich podzbiorem, który można wykonać w dowolnej kolejności (przednie i tylne siedzenia mogą być instalowane w dowolnej kolejności).

Moja logika używa obecnie Reflection dla uproszczenia implementacji.

To znaczy, gdy mam CarWithoutUpholstery, obiekt sprawdza się pod kątem metod o nazwie performSomething (). W tym momencie wykonuje wszystkie te metody:

myObject.perform001SomeOperation();
myObject.perform002SomeOtherOperation();
...

podczas sprawdzania błędów i innych rzeczy. Chociaż kolejność operacji jest nieistotna, przypisałem porządek leksykograficzny na wypadek, gdyby kiedykolwiek odkryłem, że jakiś porządek jest ważny. Jest to sprzeczne z YAGNI , ale kosztowało bardzo niewiele - prosty sort () - i może zaoszczędzić ogromnej ilości zmian nazw metod (lub wprowadzenia innych metod wykonywania testów, np. Szeregu metod).

Inny przykład

Powiedzmy, że zamiast budować samochód muszę sporządzić komuś raport Tajnej Policji i przekazać go mojemu Złemu Władcy . Moim ostatnim obiektem będzie ReadyReport. Aby go zbudować, zaczynam od zebrania podstawowych informacji (imię, nazwisko, małżonek ...). To moja faza A. W zależności od tego, czy jest małżonek, czy nie, może być konieczne przejście do faz B1 lub B2 i zebranie danych dotyczących seksualności jednej lub dwóch osób. Składa się z kilku różnych zapytań do różnych złych stworów kontrolujących życie nocne, kamery uliczne, paragony sprzedaży w sklepach erotycznych i co tam nie. I tak dalej i tak dalej.

Jeśli ofiara nie ma rodziny, nie wejdę nawet w fazę GetInformationAboutFamily, ale jeśli to zrobię, nie ma znaczenia, czy najpierw skieruję cel na ojca, matkę lub rodzeństwo (jeśli w ogóle). Ale nie mogę tego zrobić, jeśli nie wykonałem FamilyStatusCheck, który dlatego należy do wcześniejszej fazy.

Wszystko działa cudownie ...

  • jeśli potrzebuję dodatkowej operacji, muszę tylko dodać metodę prywatną,
  • jeśli operacja jest wspólna dla kilku faz, mogę ją odziedziczyć z nadklasy,
  • operacje są proste i niezależne. Żadna inna nie wymaga żadnej wartości z jednej operacji (operacje, które są wykonywane w innej fazie),
  • obiekty w dalszej linii nie muszą przeprowadzać wielu testów, ponieważ nie mogłyby nawet istnieć, gdyby ich twórcy nie zweryfikowali tych warunków w pierwszej kolejności. To znaczy, umieszczając wstawki w desce rozdzielczej, czyszcząc deskę rozdzielczą i weryfikując deskę rozdzielczą, nie muszę weryfikować, czy deska rozdzielcza faktycznie tam jest .
  • pozwala na łatwe testowanie. Mogę z łatwością wyśmiewać częściowy obiekt i uruchomić na nim dowolną metodę, a wszystkie operacje są deterministycznymi czarnymi skrzynkami.

...ale...

Problem pojawił się, gdy dodałem ostatnią operację w jednym z moich obiektów metod, co spowodowało, że ogólny moduł przekroczył obowiązkowy indeks złożoności („mniej niż N metod prywatnych”).

Już wziąłem sprawę na górę i zasugerowałem, że w tym przypadku bogactwo prywatnych metod nie jest oznaką katastrofy. Złożoność jest obecna, ale istnieje, ponieważ operacja jest złożona, a tak naprawdę nie jest tak złożona - jest po prostu długa .

Korzystając z przykładu Złego Władcy, moim problemem jest to, że Władca Zła (znany również jako Ten, Który Nie Będzie Zaprzeczony ), który poprosił o wszystkie informacje dietetyczne, moje Sługi Dietetyczne mówią mi, że muszę zapytać restauracje, aneksy kuchenne, ulicznych sprzedawców, nielicencjonowanych sprzedawców ulicznych, szklarnię właściciele itp. oraz Zły (pod) Overlord - znany jako Ten, Który Również Nie Będzie Zaprzeczony - narzekając, że wykonuję zbyt wiele zapytań w fazie GetDietaryInformation.

Uwaga : zdaję sobie sprawę, że z kilku punktów widzenia nie jest to wcale problemem (ignorowanie możliwych problemów z wydajnością itp.). Wszystko, co się dzieje, polega na tym, że określona metryka jest niezadowolona i jest to uzasadnione.

Co myślę, że mógłbym zrobić

Oprócz pierwszego wszystkie te opcje są wykonalne i, moim zdaniem, możliwe do obrony.

  • Potwierdziłem, że mogę być podstępny i zadeklarować połowę moich metod protected. Ale wykorzystywałbym słabość procedury testowej i poza usprawiedliwieniem się, gdy mnie przyłapano, nie podoba mi się to. Ponadto jest to miara zatrzymująca. Co się stanie, jeśli liczba wymaganych operacji podwoi się? Mało prawdopodobne, ale co wtedy?
  • Mogę dowolnie podzielić tę fazę na AnnealedObjectAlpha, AnnealedObjectBravo i AnnealedObjectCharlie, a jedną trzecią operacji wykonuję na każdym etapie. Mam wrażenie, że to faktycznie dodaje złożoności (więcej klas N-1), bez żadnej korzyści poza zaliczeniem testu. Mogę oczywiście stwierdzić, że CarWithFrontSeatsInstalled i CarWithAllSeatsInstalled to logicznie kolejne etapy. Ryzyko, że metoda Bravo będzie później wymagana przez Alphę, jest niewielkie, a nawet mniejsze, jeśli dobrze ją zagram. Ale nadal.
  • Mogę łączyć różne operacje, zdalnie podobne, w jednym. performAllSeatsInstallation(). Jest to tylko miara zatrzymująca i zwiększa złożoność pojedynczej operacji. Jeśli kiedykolwiek będę musiał wykonać operacje A i B w innej kolejności i zapakowałem je w E = (A + C) i F (B + D), będę musiał rozdzielić E i F i przetasować kod .
  • Mogę użyć szeregu funkcji lambda i całkowicie ominąć czek, ale uważam to za niezręczne. Jest to jednak jak dotąd najlepsza alternatywa. Pozbyłby się refleksji. Dwa problemy, które mam, polegają na tym, że prawdopodobnie zostałbym poproszony o przepisanie wszystkich obiektów metod, nie tylko hipotetycznych CarWithEngineInstalled, i chociaż byłoby to bardzo dobre bezpieczeństwo pracy, to tak naprawdę wcale nie jest atrakcyjne; oraz że sprawdzanie pokrycia kodu ma problemy z lambdami (które można rozwiązać, ale nadal ).

Więc...

  • Jak myślisz, która opcja jest dla mnie najlepsza?
  • Czy jest lepszy sposób, którego nie rozważałem? ( może lepiej przyjdę czysty i zapytam bezpośrednio, co to jest? )
  • Czy ten projekt jest beznadziejnie wadliwy i czy lepiej przyznać się do porażki i rowy - tej architektury? Nie jest to dobre dla mojej kariery, ale czy pisanie źle zaprojektowanego kodu byłoby lepsze w dłuższej perspektywie?
  • Czy mój obecny wybór jest w rzeczywistości One True Way i muszę walczyć o zainstalowanie lepszej jakości wskaźników (i / lub oprzyrządowania)? Do tej ostatniej opcji potrzebuję referencji ... Nie mogę po prostu machnąć ręką na @PHB podczas szemrania. To nie są te parametry, których szukasz . Nie ważne jak bardzo chciałbym być w stanie
LSerni
źródło
3
Możesz też zignorować ostrzeżenie „przekroczona maksymalna złożoność”.
Robert Harvey
Gdybym mógł, zrobiłbym to. Jakoś ta metryka nabrała własnej świętości. Wciąż próbuję przekonać PTB do zaakceptowania ich jako przydatnego narzędzia, ale tylko narzędzia. Mogę jeszcze odnieść sukces ...
LSerni,
Czy operacje faktycznie konstruują obiekt, czy po prostu używasz metafory linii montażowej, ponieważ dzieli ona określoną właściwość, o której wspomniałeś (wiele operacji z częściową kolejnością zależności)? Istotne jest to, że ta pierwsza sugeruje wzorzec konstruktora, podczas gdy druga może sugerować jakiś inny wzorzec z większą ilością szczegółów.
outis
2
Tyrania mierników. Metryki są przydatnymi wskaźnikami, ale egzekwowanie ich przy ignorowaniu, dlaczego ta metryka jest używana, nie jest pomocne.
Jaydee,
2
Wyjaśnij ludziom na górze różnicę między niezbędną złożonością a przypadkową złożonością. pl.wikipedia.org/wiki/No_Silver_Bullet Jeśli masz pewność, że złożoność, która łamie regułę, jest niezbędna, to jeśli refaktoryzujesz według programistów.stackexchange.com /a/297414/51948 , prawdopodobnie jeździsz wokół reguły i rozprzestrzeniasz się ze złożoności. Jeśli kapsułkowanie rzeczy w fazy nie jest arbitralne (fazy odnoszą się do problemu), a przeprojektowanie zmniejsza obciążenie poznawcze dla deweloperów utrzymujących kod, warto to zrobić.
Fuhrmanator

Odpowiedzi:

15

Długa sekwencja operacji wydaje się, że powinna to być jej własna klasa, która następnie potrzebuje dodatkowych obiektów do wykonania swoich obowiązków. Wygląda na to, że jesteś blisko tego rozwiązania. Tę długą procedurę można podzielić na wiele etapów lub faz. Każdy krok lub faza może być własną klasą, która ujawnia metodę publiczną. Chcesz, aby każda faza implementowała ten sam interfejs. Następnie twoja linia montażowa śledzi listę faz.

Aby skorzystać z przykładu montażu samochodu, wyobraź sobie Carklasę:

public class Car
{
    public IChassis Chassis { get; private set; }
    public Dashboard { get; private set; }
    public IList<Seat> Seats { get; private set; }

    public Car()
    {
        Seats = new List<Seat>();
    }

    public void AddChassis(IChassis chassis)
    {
        Chassis = chassis;
    }

    public void AddDashboard(Dashboard dashboard)
    {
        Dashboard = dashboard;
    }
}

Zamierzam pominąć implementacje niektórych składników, takich jak IChassis, Seati Dashboarddla zwięzłości.

Nie Carjest odpowiedzialny za sam budynek. Linia montażowa jest, więc podsumujmy to w klasie:

public class CarAssembler
{
    protected List<IAssemblyPhase> Phases { get; private set; }

    public CarAssembler()
    {
        Phases = new List<IAssemblyPhase>()
        {
            new ChassisAssemblyPhase(),
            new DashboardAssemblyPhase(),
            new SeatAssemblyPhase()
        };
    }

    public void Assemble(Car car)
    {
        foreach (IAssemblyPhase phase in Phases)
        {
            phase.Assemble(car);
        }
    }
}

Ważnym rozróżnieniem jest tutaj to, że CarAssembler ma listę implementowanych obiektów fazy montażu IAssemblyPhase. Masz teraz do czynienia z jawnymi metodami publicznymi, co oznacza, że ​​każdą fazę można przetestować samodzielnie, a twoje narzędzie do poprawy jakości kodu jest szczęśliwsze, ponieważ nie upychasz się zbytnio w jedną klasę. Proces montażu jest również bardzo prosty. Po prostu obejrzyj fazy i zadzwoń Assembledo samochodu. Gotowy. IAssemblyPhaseInterfejs jest również martwe proste tylko z jednej metody publiczne, które odbywają obiektu samochodem:

public interface IAssemblyPhase
{
    void Assemble(Car car);
}

Teraz potrzebujemy naszych konkretnych klas implementujących ten interfejs dla każdej konkretnej fazy:

public class ChassisAssemblyPhase : IAssemblyPhase
{
    public void Assemble(Car car)
    {
        car.AddChassis(new UnibodyChassis());
    }
}

public class DashboardAssemblyPhase : IAssemblyPhase
{
    public void Assemble(Car car)
    {
        Dashboard dashboard = new Dashboard();

        dashboard.AddComponent(new Speedometer());
        dashboard.AddComponent(new FuelGuage());
        dashboard.Trim = DashboardTrimType.Aluminum;

        car.AddDashboard(dashboard);
    }
}

public class SeatAssemblyPhase : IAssemblyPhase
{
    public void Assemble(Car car)
    {
        car.Seats.Add(new Seat());
        car.Seats.Add(new Seat());
        car.Seats.Add(new Seat());
        car.Seats.Add(new Seat());
    }
}

Każda faza z osobna może być dość prosta. Niektóre są tylko jedną wkładką. Niektóre mogą być oślepiające, dodając cztery fotele, a inne muszą skonfigurować deskę rozdzielczą przed dodaniem jej do samochodu. Złożoność tego długotrwałego procesu dzieli się na wiele klas wielokrotnego użytku, które można łatwo przetestować.

Nawet jeśli powiedziałeś, że kolejność nie ma teraz znaczenia, może w przyszłości.

Aby to zakończyć, spójrzmy na proces montażu samochodu:

Car car = new Car();
CarAssembler assemblyLine = new CarAssembler();

assemblyLine.Assemble(car);

Możesz podklasować CarAssembler, aby złożyć konkretny rodzaj samochodu lub ciężarówki. Każda faza to jedna jednostka w większej całości, co ułatwia ponowne użycie kodu.


Czy ten projekt jest beznadziejnie wadliwy i czy lepiej przyznać się do porażki i rowy - tej architektury? Nie jest to dobre dla mojej kariery, ale czy pisanie źle zaprojektowanego kodu byłoby lepsze w dłuższej perspektywie?

Jeśli decydowanie o tym, co napisałem, musi zostać przepisane na czarno, to byłbym czarnym znakiem w mojej karierze, to bym teraz pracował jako koparka rowów, a ty nie powinieneś korzystać z moich rad. :)

Inżynieria oprogramowania jest wiecznym procesem uczenia się, aplikowania, a następnie ponownego uczenia się. To, że zdecydujesz się przepisać kod, nie oznacza, że ​​zostaniesz zwolniony. Gdyby tak było, nie byłoby inżynierów oprogramowania.

Czy mój obecny wybór jest w rzeczywistości One True Way i muszę walczyć o zainstalowanie lepszej jakości wskaźników (i / lub oprzyrządowania)?

Naszkicowałeś nie Jedną Prawdziwą Drogę, ale pamiętaj, że metryki kodu również nie są Jedną Prawdziwą Drogą. Dane kodu należy traktować jako ostrzeżenia. Mogą wskazywać na problemy konserwacyjne w przyszłości. Są to wytyczne, a nie prawa. Gdy narzędzie metryk kodu wskazuje coś, najpierw zbadam kod, aby sprawdzić, czy poprawnie implementuje on zasady SOLID . Wielokrotnie refaktoryzacja kodu, aby był bardziej SOLIDNY, zaspokoi narzędzia metryk kodu. Czasem tak nie jest. Musisz wziąć te wskaźniki indywidualnie dla każdego przypadku.

Greg Burghardt
źródło
1
Tak, znacznie lepiej niż mieć jedną klasę boga.
BЈовић
Takie podejście działa, ale przede wszystkim dlatego, że można uzyskać elementy do samochodu bez parametrów. Co jeśli musisz je przekazać? Następnie możesz to wszystko wyrzucić przez okno i całkowicie przemyśleć procedurę, ponieważ tak naprawdę nie masz fabryki samochodów o 150 parametrach, to szalone.
Andy,
@DavidPacker: Nawet jeśli musisz sparametryzować wiele rzeczy, możesz to również zawrzeć w jakiejś klasie Config, którą przekazujesz do konstruktora obiektu „asembler”. W tym przykładzie samochodu można dostosować kolor farby, kolor wnętrza i materiały, pakiet wykończenia, silnik i wiele innych, ale niekoniecznie będziesz miał 150 dostosowań. Prawdopodobnie będziesz miał kilkanaście, które nadają się do obiektu opcji.
Greg Burghardt
Ale tylko dodanie konfiguracji przeczyłoby celowi pięknej pętli for, co jest ogromnym wstydem, ponieważ musiałbyś przeanalizować konfigurację i przypisać ją do odpowiednich metod, które w zamian dałyby ci odpowiednie obiekty. Prawdopodobnie skończyłbyś z czymś podobnym do oryginalnego projektu.
Andy
Widzę. Zamiast jednej klasy i pięćdziesięciu metod, faktycznie miałbym pięćdziesiąt klas lean asemblera i jedną klasę aranżacyjną. Ostatecznie wynik wydaje się taki sam, również pod względem wydajności, ale układ kodu jest czystszy. Lubię to. Nieco trudniej będzie dodawać operacje (będę musiał skonfigurować je w swojej klasie, a także poinformować klasę orkiestrującą; nie widzę łatwego wyjścia z tej konieczności), ale nadal bardzo mi się podoba. Daj mi kilka dni na zbadanie tego podejścia.
LSerni,
1

Nie wiem, czy jest to możliwe, ale zastanawiałbym się nad zastosowaniem podejścia bardziej zorientowanego na dane. Chodzi o to, że przechwytujesz pewne (deklaratywne) wyrażenie reguł i ograniczeń, operacji, zależności, stanu itp. Następnie twoje klasy i kod są bardziej ogólne, zorientowane na przestrzeganie reguł, inicjowanie bieżącego stanu instancji, wykonywanie operacji w celu zmień stan, używając deklaratywnego przechwytywania reguł i zmian stanu zamiast używać kodu bardziej bezpośrednio. Można użyć deklaracji danych w języku programowania lub niektórych narzędzi DSL albo reguł lub silnika logicznego.

Erik Eidt
źródło
Niektóre operacje są faktycznie realizowane w ten sposób (jako listy reguł). Na przykład w przypadku samochodu, instalując skrzynkę z bezpiecznikami, lampkami i wtyczkami, tak naprawdę nie używam trzech różnych metod, ale tylko jedną, która ogólnie pasuje do dwóch pinów elektrycznych rzeczy na miejscu i bierze Listę trzech list bezpieczników , światła i wtyczki. Ogromna większość innych metod niestety nie jest łatwo poddać się takiemu podejściu.
LSerni,
1

W jakiś sposób problem przypomina mi zależności. Chcesz samochód w określonej konfiguracji. Podobnie jak Greg Burghardt, chodziłbym z obiektami / klasami dla każdego kroku / przedmiotu /….

Każdy krok deklaruje, co dodaje do miksu (co to provides ) (może to być wiele rzeczy), ale także to, czego wymaga.

Następnie definiujesz gdzieś ostateczną wymaganą konfigurację i masz algorytm, który sprawdza, czego potrzebujesz, i decyduje, które kroki należy wykonać / jakie części należy dodać / jakie dokumenty należy zebrać.

Algorytmy rozwiązywania zależności nie są trudne. Najprostszy przypadek polega na tym, że każdy krok zapewnia jedną rzecz, a żadne dwie rzeczy nie zapewniają tego samego, a zależności są proste (brak „lub” w zależnościach). W bardziej złożonych przypadkach: wystarczy spojrzeć na narzędzie takie jak debian apt czy coś takiego.

Wreszcie, zbudowanie samochodu ogranicza się do możliwych kroków i pozwala CarBuilder na określenie wymaganych kroków.

Jedną ważną rzeczą jest jednak to, że musisz znaleźć sposób, aby CarBuilder wiedział o wszystkich krokach. Spróbuj to zrobić przy użyciu pliku konfiguracyjnego. Dodanie dodatkowego kroku wymagałoby jedynie dodania pliku konfiguracyjnego. Jeśli potrzebujesz logiki, spróbuj dodać DSL do pliku konfiguracyjnego. Jeśli wszystko inne zawiedzie, utkniesz definiując je w kodzie.

Sjoerd Job Postmus
źródło
0

Kilka rzeczy, o których wspominasz, są dla mnie „złe”

„Moja logika używa obecnie Reflection dla uproszczenia implementacji”

Naprawdę nie ma na to usprawiedliwienia. czy naprawdę używasz porównywania ciągów nazw metod, aby określić, co uruchomić ?! jeśli zmienisz kolejność, będziesz musiał zmienić nazwę metody ?!

„Operacje w jednej fazie są już niezależne”

Jeśli są naprawdę od siebie niezależni, sugeruje to, że twoja klasa przyjęła na siebie więcej niż jedną odpowiedzialność. Wydaje mi się, że oba twoje przykłady nadają się do jednego singla

MyObject.AddComponent(IComponent component) 

podejście, które pozwoliłoby ci podzielić logikę dla każdej operacji na własną klasę lub klasy.

W rzeczywistości wyobrażam sobie, że nie są naprawdę niezależne, wyobrażam sobie, że każdy bada stan obiektu i modyfikuje go, lub przynajmniej dokonuje pewnego rodzaju weryfikacji przed rozpoczęciem.

W takim przypadku możesz:

  1. Wyrzuć OOP przez okno i masz usługi, które działają na narażonych danych w twojej klasie, tj.

    Service.AddDoorToCar (samochód samochodowy)

  2. Mieć klasę konstruktora, która konstruuje obiekt, najpierw gromadząc potrzebne dane i przekazując ukończony obiekt, tj.

    Car = CarBuilder.WithDoor (). WithDoor (). WithWheels ();

(logika withX może zostać ponownie podzielona na podbudowy)

  1. Zastosuj podejście funkcjonalne i przekaż funkcje / delgaty do metody modyfikacji

    Car.Modify ((c) => c.doors ++);

Ewan
źródło
Ewan powiedział: „Wyrzuć OOP przez okno i masz usługi, które działają na ujawnionych danych w twojej klasie” --- Jak to nie jest zorientowane obiektowo?
Greg Burghardt,
?? to nie jest, to nie jest opcja OO
Ewan
Używasz obiektów, co oznacza, że ​​jest „obiektowy”. To, że nie możesz zidentyfikować wzorca programowania dla twojego kodu, nie oznacza, że ​​nie jest on zorientowany obiektowo. Zasady SOLID są dobrą zasadą. Jeśli masz zaimplementowane niektóre lub wszystkie zasady SOLID, jest to obiektowe. Naprawdę, jeśli używasz obiektów, a nie tylko przekazujesz struktury do różnych procedur lub metod statycznych, jest to obiektowe. Istnieje różnica między „kodem obiektowym” a „dobrym kodem”. Możesz pisać źle zorientowany obiektowo kod.
Greg Burghardt,
jego „nie OO”, ponieważ ujawniasz dane tak, jakby to była struktura i działałeś na nich w oddzielnej klasie jak w procedurze
Ewan
tbh Dodałem tylko wiodący komentarz, aby powstrzymać nieuniknione „to nie jest OOP!” komentarze
Ewan