Jaki jest najlepszy sposób na refaktoryzację metody, która ma zbyt wiele (6+) parametrów?

Sporadycznie spotykam metody o niewygodnej liczbie parametrów. Najczęściej wydają się być konstruktorami. Wygląda na to, że powinien być lepszy sposób, ale nie widzę, co to jest.

return new Shniz(foo, bar, baz, quux, fred, wilma, barney, dino, donkey)

Myślałem o użyciu struktur do reprezentowania listy parametrów, ale wydaje się, że przenosi to problem z jednego miejsca w drugie i tworzy w procesie inny typ.

ShnizArgs args = new ShnizArgs(foo, bar, baz, quux, fred, wilma, barney, dino, donkey)
return new Shniz(args);

Więc to nie wygląda na poprawę. Więc jakie jest najlepsze podejście?

Najlepszym sposobem byłoby znalezienie sposobów na pogrupowanie argumentów. Zakłada się, i naprawdę działa tylko wtedy, gdy skończyłoby się z wieloma „grupami” argumentów.

Na przykład, jeśli przekazujesz specyfikację prostokąta, możesz przekazać x, y, szerokość i wysokość lub możesz po prostu przekazać obiekt prostokątny, który zawiera x, y, szerokość i wysokość.

Szukaj takich rzeczy podczas refaktoryzacji, aby nieco to uporządkować. Jeśli naprawdę nie można połączyć argumentów, zacznij sprawdzać, czy nie naruszyłeś zasady pojedynczej odpowiedzialności.

Zakładam, że masz na myśli C # . Niektóre z tych rzeczy odnoszą się również do innych języków.

Masz kilka możliwości:

przełącz się z konstruktora na ustawiające właściwości . Może to uczynić kod bardziej czytelnym, ponieważ dla czytelnika jest oczywiste, która wartość odpowiada jakim parametrom. Składnia inicjatora obiektu sprawia, że ​​wygląda to ładnie. Jest również prosty w implementacji, ponieważ możesz po prostu użyć właściwości generowanych automatycznie i pominąć pisanie konstruktorów.

class C
{
    public string S { get; set; }
    public int I { get; set; }
}

new C { S = "hi", I = 3 };

Jednak tracisz niezmienność i tracisz możliwość upewnienia się, że wymagane wartości są ustawione przed użyciem obiektu w czasie kompilacji.

Wzorzec konstruktora .

Pomyśl o relacji między stringa StringBuilder. Możesz to otrzymać na własne zajęcia. Lubię implementować go jako klasę zagnieżdżoną, więc klasa Cma powiązaną klasę C.Builder. Podoba mi się też płynny interfejs kreatora. Zrobione dobrze, możesz uzyskać taką składnię:

C c = new C.Builder()
    .SetX(4)    // SetX is the fluent equivalent to a property setter
    .SetY("hello")
    .ToC();     // ToC is the builder pattern analog to ToString()

// Modify without breaking immutability
c = c.ToBuilder().SetX(2).ToC();

// Still useful to have a traditional ctor:
c = new C(1, "...");

// And object initializer syntax is still available:
c = new C.Builder { X = 4, Y = "boing" }.ToC();

Mam skrypt PowerShell, który pozwala mi wygenerować kod konstruktora, aby zrobić to wszystko, gdzie dane wejściowe wyglądają następująco:

class C {
    field I X
    field string Y
}

Więc mogę generować w czasie kompilacji. partialklasy pozwalają mi rozszerzyć zarówno klasę główną, jak i konstruktora bez modyfikowania wygenerowanego kodu.

Refaktoryzacja „Wprowadź obiekt parametru” . Zobacz katalog refaktoryzacji . Pomysł polega na tym, że bierzesz część przekazywanych parametrów i umieszczasz je w nowym typie, a następnie przekazujesz instancję tego typu. Jeśli zrobisz to bez zastanowienia, wrócisz tam, gdzie zacząłeś:

new C(a, b, c, d);

staje się

new C(new D(a, b, c, d));

Jednak to podejście ma największy potencjał, aby wywrzeć pozytywny wpływ na Twój kod. Więc kontynuuj, wykonując następujące kroki:

1. Poszukaj podzbiorów parametrów, które mają sens razem. Samo bezmyślne grupowanie wszystkich parametrów funkcji nie daje wiele; celem jest stworzenie sensownych grup. Dowiesz się, że masz rację, gdy nazwa nowego typu będzie oczywista.

2. Poszukaj innych miejsc, w których te wartości są używane razem, i użyj tam również nowego typu. Są szanse, że kiedy znajdziesz dobry nowy typ dla zestawu wartości, których już używasz w każdym miejscu, ten nowy typ będzie miał również sens we wszystkich tych miejscach.

3. Poszukaj funkcji, która znajduje się w istniejącym kodzie, ale należy do nowego typu.

Na przykład może zobaczysz kod, który wygląda następująco:

bool SpeedIsAcceptable(int minSpeed, int maxSpeed, int currentSpeed)
{
    return currentSpeed >= minSpeed & currentSpeed < maxSpeed;
}

Można podjąć minSpeedi maxSpeedparametry i umieścić je w nowym rodzaju:

class SpeedRange
{
   public int Min;
   public int Max;
}

bool SpeedIsAcceptable(SpeedRange sr, int currentSpeed)
{
    return currentSpeed >= sr.Min & currentSpeed < sr.Max;
}

To jest lepsze, ale aby naprawdę wykorzystać nowy typ, przenieś porównania do nowego typu:

class SpeedRange
{
   public int Min;
   public int Max;

   bool Contains(int speed)
   {
       return speed >= min & speed < Max;
   }
}

bool SpeedIsAcceptable(SpeedRange sr, int currentSpeed)
{
    return sr.Contains(currentSpeed);
}

A teraz gdzieś SpeedIsAcceptable()dochodzimy : implementacja now mówi o tym, co masz na myśli, i masz użyteczną klasę wielokrotnego użytku. (Następnym oczywistym krokiem jest zrobienie SpeedRangetego Range<Speed>.)

Jak widać, wprowadzenie obiektu parametru było dobrym początkiem, ale jego prawdziwą wartością było to, że pomogło nam odkryć przydatny typ, którego brakowało w naszym modelu.

Jeśli jest to konstruktor, szczególnie jeśli istnieje wiele przeciążonych wariantów, powinieneś spojrzeć na wzorzec Builder:

Foo foo = new Foo()
          .configBar(anything)
          .configBaz(something, somethingElse)
          // and so on

Jeśli jest to normalna metoda, powinieneś pomyśleć o relacjach między przekazywanymi wartościami i być może utworzyć obiekt transferu.

Klasyczną odpowiedzią na to jest użycie klasy do hermetyzacji niektórych lub wszystkich parametrów. W teorii brzmi to świetnie, ale jestem typem faceta, który tworzy zajęcia dla pojęć mających znaczenie w tej dziedzinie, więc nie zawsze łatwo jest zastosować tę radę.

Np. Zamiast:

driver.connect(host, user, pass)

Możesz użyć

config = new Configuration()
config.setHost(host)
config.setUser(user)
config.setPass(pass)
driver.connect(config)

YMMV

Cytat z książki Fowlera i Becka: „Refaktoryzacja”

Długa lista parametrów

We wczesnych dniach programowania uczono nas, aby przekazywać jako parametry wszystko, czego potrzebuje procedura. Było to zrozumiałe, ponieważ alternatywą były dane globalne, a dane globalne są złe i zwykle bolesne. Przedmioty zmieniają tę sytuację, ponieważ jeśli nie masz czegoś, czego potrzebujesz, zawsze możesz poprosić inny przedmiot, aby to dla ciebie zrobił. Zatem w przypadku obiektów nie przekazujesz wszystkiego, czego potrzebuje metoda; zamiast tego zaliczasz wystarczająco dużo, aby metoda mogła dotrzeć do wszystkiego, czego potrzebuje. Wiele potrzebnych metod jest dostępnych w klasie hosta metody. W programach zorientowanych obiektowo listy parametrów są zwykle dużo mniejsze niż w tradycyjnych programach. To dobrze, ponieważ długie listy parametrów są trudne do zrozumienia, ponieważ stają się niespójne i trudne w użyciu, i ponieważ ciągle je zmieniasz, ponieważ potrzebujesz więcej danych. Większość zmian jest usuwana przez przekazywanie obiektów, ponieważ istnieje większe prawdopodobieństwo, że będziesz musiał wykonać tylko kilka żądań, aby uzyskać nową część danych. Użyj opcji Zastąp parametr metodą, gdy możesz uzyskać dane w jednym parametrze, wysyłając żądanie obiektu, o którym już wiesz. Ten obiekt może być polem lub innym parametrem. Użyj opcji Zachowaj cały obiekt, aby pobrać zbiór danych zebranych z obiektu i zastąpić go samym obiektem. Jeśli masz kilka elementów danych bez obiektu logicznego, użyj funkcji wprowadzenia obiektu parametru. Jest jeden ważny wyjątek od dokonywania tych zmian. Dzieje się tak, gdy jawnie nie chcesz tworzyć zależności od wywoływanego obiektu do większego obiektu. W takich przypadkach rozpakowanie danych i przesłanie ich jako parametrów jest rozsądne, ale zwróć uwagę na ból. Jeśli lista parametrów jest zbyt długa lub zmienia się zbyt często, musisz przemyśleć strukturę zależności.

Nie chcę brzmieć jak mądry crack, ale powinieneś także sprawdzić, czy dane, które przekazujesz, naprawdę powinny zostać przekazane: Przekazywanie rzeczy konstruktorowi (lub metodzie w tym zakresie) pachnie trochę jak mały nacisk na zachowanie obiektu.

Nie zrozumcie mnie źle: Metody i konstruktorzy będą mieć dużo parametrów czasem. Ale kiedy napotkasz, spróbuj rozważyć hermetyzację danych za pomocą zachowania zamiast tego .

Ten rodzaj zapachu (skoro mówimy o refaktoryzacji, to okropne słowo wydaje się odpowiednie ...) może być również wykrywany dla obiektów, które mają wiele (czytaj: dowolnych) właściwości lub metody pobierające / ustawiające.

Kiedy widzę długie listy parametrów, moje pierwsze pytanie dotyczy tego, czy ta funkcja lub obiekt robi za dużo. Rozważać:

EverythingInTheWorld earth=new EverythingInTheWorld(firstCustomerId,
  lastCustomerId,
  orderNumber, productCode, lastFileUpdateDate,
  employeeOfTheMonthWinnerForLastMarch,
  yearMyHometownWasIncorporated, greatGrandmothersBloodType,
  planetName, planetSize, percentWater, ... etc ...);

Oczywiście ten przykład jest celowo śmieszny, ale widziałem wiele prawdziwych programów z przykładami tylko nieco mniej absurdalnymi, w których jedna klasa jest używana do przechowywania wielu ledwo powiązanych lub niepowiązanych rzeczy, najwyraźniej tylko dlatego, że ten sam program wywołujący wymaga obu lub ponieważ programista pomyślał o obu jednocześnie. Czasami prostym rozwiązaniem jest podzielenie klasy na wiele części, z których każda robi swoje.

Nieco bardziej skomplikowane jest sytuacja, gdy klasa naprawdę musi zajmować się wieloma logicznymi rzeczami, takimi jak zamówienie klienta i ogólne informacje o kliencie. W takich przypadkach stwórz klasę dla klienta i klasę na zamówienie i pozwól im rozmawiać ze sobą w razie potrzeby. Więc zamiast:

 Order order=new Order(customerName, customerAddress, customerCity,
   customerState, customerZip,
   orderNumber, orderType, orderDate, deliveryDate);

Moglibyśmy mieć:

Customer customer=new Customer(customerName, customerAddress,
  customerCity, customerState, customerZip);
Order order=new Order(customer, orderNumber, orderType, orderDate, deliveryDate);

Chociaż oczywiście wolę funkcje, które przyjmują tylko 1, 2 lub 3 parametry, czasami musimy zaakceptować, że realistycznie rzecz biorąc, ta funkcja zajmuje sporo, a sama liczba nie powoduje tak naprawdę złożoności. Na przykład:

Employee employee=new Employee(employeeId, firstName, lastName,
  socialSecurityNumber,
  address, city, state, zip);

Tak, jest to kilka pól, ale prawdopodobnie wszystko, co z nimi zrobimy, to zapisać je w rekordzie bazy danych lub wrzucić na ekran lub coś takiego. Nie ma tu zbyt wiele przetwarzania.

Kiedy moje listy parametrów robią się długie, wolę, jeśli mogę nadać polom różne typy danych. Na przykład, gdy widzę funkcję taką jak:

void updateCustomer(String type, String status,
  int lastOrderNumber, int pastDue, int deliveryCode, int birthYear,
  int addressCode,
  boolean newCustomer, boolean taxExempt, boolean creditWatch,
  boolean foo, boolean bar);

A potem widzę, jak nazywa się to:

updateCustomer("A", "M", 42, 3, 1492, 1969, -7, true, false, false, true, false);

Martwię się. Patrząc na wezwanie, nie jest wcale jasne, co oznaczają te wszystkie tajemnicze liczby, kody i flagi. To tylko prośba o błędy. Programista może łatwo się pomylić co do kolejności parametrów i przypadkowo przełączyć dwa, a jeśli są tego samego typu danych, kompilator po prostu by to zaakceptował. Wolałbym raczej mieć podpis, w którym wszystkie te rzeczy są wyliczeniami, więc wywołanie przechodzi w takich rzeczach, jak Type.ACTIVE zamiast „A” i CreditWatch.NO zamiast „false” itp.

Jeśli niektóre parametry konstruktora są opcjonalne, sensowne jest użycie konstruktora, który pobierałby wymagane parametry w konstruktorze i miał metody dla opcjonalnych, zwracających konstruktora, do użycia w następujący sposób:

return new Shniz.Builder(foo, bar).baz(baz).quux(quux).build();

Szczegóły tego są opisane w Effective Java, 2nd Ed., Str. 11. Jeśli chodzi o argumenty metod, ta sama książka (s. 189) opisuje trzy podejścia do skracania list parametrów:

• Podziel metodę na wiele metod, które pobierają mniej argumentów
• Utwórz statyczne klasy pomocnicze reprezentujące grupy parametrów, tj. Przekaż DinoDonkeyzamiast dinoidonkey
• Jeśli parametry są opcjonalne, powyższy konstruktor może zostać zaadoptowany do metod, definiując obiekt dla wszystkich parametrów, ustawiając wymagane, a następnie wywołując na nim jakąś metodę wykonania

Użyłbym domyślnego konstruktora i ustawiaczy właściwości. C # 3.0 ma niezłą składnię, dzięki której można to zrobić automagicznie.

return new Shniz { Foo = foo,
                   Bar = bar,
                   Baz = baz,
                   Quuz = quux,
                   Fred = fred,
                   Wilma = wilma,
                   Barney = barney,
                   Dino = dino,
                   Donkey = donkey
                 };

Ulepszenie kodu polega na uproszczeniu konstruktora i braku konieczności obsługi wielu metod do obsługi różnych kombinacji. Składnia „wywoływania” jest nadal trochę „rozwlekła”, ale tak naprawdę nie jest gorsza niż ręczne wywoływanie ustawników właściwości.

Nie dostarczyłeś wystarczających informacji, aby uzasadnić dobrą odpowiedź. Długa lista parametrów nie jest z natury zła.

Shniz (foo, bar, baz, quux, fred, wilma, barney, dino, donkey)

można interpretować jako:

void Shniz(int foo, int bar, int baz, int quux, int fred, 
           int wilma, int barney, int dino, int donkey) { ...

W takim przypadku o wiele lepiej jest utworzyć klasę do hermetyzacji parametrów, ponieważ nadajesz znaczenie różnym parametrom w sposób, który kompilator może sprawdzić, a także wizualnie ułatwiając odczytanie kodu. Ułatwia również późniejsze czytanie i refaktoryzację.

// old way
Shniz(1,2,3,2,3,2,1,2);
Shniz(1,2,2,3,3,2,1,2); 

//versus
ShnizParam p = new ShnizParam { Foo = 1, Bar = 2, Baz = 3 };
Shniz(p);

Alternatywnie, jeśli miałeś:

void Shniz(Foo foo, Bar bar, Baz baz, Quux quux, Fred fred, 
           Wilma wilma, Barney barney, Dino dino, Donkey donkey) { ...

To zupełnie inny przypadek, ponieważ wszystkie obiekty są różne (i prawdopodobnie nie zostaną pomieszane). Zgodzono się, że jeśli wszystkie obiekty są potrzebne i wszystkie są różne, tworzenie klasy parametrów nie ma sensu.

Dodatkowo, czy niektóre parametry są opcjonalne? Czy istnieją nadpisania metody (ta sama nazwa metody, ale różne sygnatury metod?) Te rodzaje szczegółów mają znaczenie dla tego, co jest najlepsze odpowiedzi.

* Torba na własność może być również przydatna, ale nie jest lepsza, biorąc pod uwagę, że nie ma podanego tła.

Jak widać, jest więcej niż jedna poprawna odpowiedź na to pytanie. Wybieraj.

Możesz spróbować zgrupować swój parametr w wiele znaczących struktur / klas (jeśli to możliwe).

Generalnie skłaniałbym się ku podejściu structs - przypuszczalnie większość tych parametrów jest w jakiś sposób powiązanych i reprezentuje stan jakiegoś elementu, który jest istotny dla twojej metody.

Jeśli zestawu parametrów nie można przekształcić w znaczący obiekt, prawdopodobnie jest to znak, że Shnizrobi za dużo, a refaktoryzacja powinna obejmować rozbicie metody na osobne zagadnienia.

Możesz handlować złożonością liniami kodu źródłowego. Jeśli sama metoda robi za dużo (nóż szwajcarski), spróbuj zmniejszyć o połowę jej zadania, tworząc inną metodę. Jeśli metoda jest prosta, tylko że wymaga zbyt wielu parametrów, to najlepszym rozwiązaniem są tzw. Obiekty parametrów.

Jeśli twój język to obsługuje, użyj nazwanych parametrów i ustaw jak najwięcej opcjonalnych (z rozsądnymi wartościami domyślnymi).

Myślę, że metoda, którą opisałeś, jest właściwą drogą. Kiedy znajduję metodę z wieloma parametrami i / lub taką, która prawdopodobnie będzie potrzebować więcej w przyszłości, zwykle tworzę obiekt ShnizParams do przejścia, tak jak opisujesz.

Co powiesz na to, aby nie ustawiać tego od razu w konstruktorach, ale robiąc to za pomocą właściwości / ustawiających ? Widziałem kilka klas .NET, które wykorzystują to podejście, na przykład Processclass:

        Process p = new Process();

        p.StartInfo.UseShellExecute = false;
        p.StartInfo.CreateNoWindow = true;
        p.StartInfo.RedirectStandardOutput = true;
        p.StartInfo.RedirectStandardError = true;
        p.StartInfo.FileName = "cmd";
        p.StartInfo.Arguments = "/c dir";
        p.Start();

Zgadzam się z podejściem przenoszenia parametrów do obiektu parametrycznego (struktury). Zamiast jednak trzymać je wszystkie w jednym obiekcie, sprawdź, czy inne funkcje używają podobnych grup parametrów. Obiekt paramater jest bardziej wartościowy, jeśli jest używany z wieloma funkcjami, w przypadku których oczekuje się, że zestaw parametrów będzie się konsekwentnie zmieniać w tych funkcjach. Może się zdarzyć, że wstawisz tylko niektóre parametry do nowego obiektu parametrów.

Jeśli masz tak wiele parametrów, istnieje prawdopodobieństwo, że metoda robi zbyt dużo, więc najpierw zajmij się tym, dzieląc metodę na kilka mniejszych metod. Jeśli po tym nadal masz zbyt wiele parametrów, spróbuj zgrupować argumenty lub przekształcić niektóre parametry w elementy członkowskie instancji.

Preferuj małe klasy / metody od dużych. Pamiętaj o zasadzie pojedynczej odpowiedzialności.

Nazwane argumenty są dobrą opcją (zakładając język, który je obsługuje) do ujednoznaczniania długich (lub nawet krótkich!) List parametrów, jednocześnie pozwalając (w przypadku konstruktorów) na niezmienność właściwości klasy bez narzucania wymogu zezwalania na jej istnienie w stanie częściowo zbudowanym.

Inną opcją, której szukałbym przy tego rodzaju refaktorze, byłyby grupy powiązanych parametrów, które mogłyby być lepiej obsługiwane jako niezależny obiekt. Używając klasy Rectangle z wcześniejszej odpowiedzi jako przykładu, konstruktor, który pobiera parametry dla x, y, wysokość i szerokość, może rozłożyć x i y do obiektu Point, umożliwiając przekazanie trzech parametrów do konstruktora Rectangle. Lub pójdź trochę dalej i ustaw dwa parametry (UpperLeftPoint, LowerRightPoint), ale to byłaby bardziej radykalna refaktoryzacja.

To zależy od tego, jakie masz argumenty, ale jeśli są to dużo wartości / opcji logicznych, może możesz użyć wyliczenia flag?

Myślę, że ten problem jest głęboko powiązany z domeną problemu, który próbujesz rozwiązać z klasą.

W niektórych przypadkach konstruktor składający się z 7 parametrów może wskazywać na złą hierarchię klas: w takim przypadku struktura / klasa pomocnicza sugerowana powyżej jest zwykle dobrym podejściem, ale wtedy również kończy się na wielu strukturach, które są po prostu zbiorami właściwości i nie rób nic pożytecznego. 8-argumentowy konstruktor może również wskazywać, że twoja klasa jest zbyt ogólna / uniwersalna, więc wymaga wielu opcji, aby była naprawdę użyteczna. W takim przypadku możesz albo refaktoryzować klasę, albo zaimplementować statyczne konstruktory, które ukrywają rzeczywiste złożone konstruktory: np. Shniz.NewBaz (foo, bar) może w rzeczywistości wywołać prawdziwy konstruktor przekazujący odpowiednie parametry.

Należy wziąć pod uwagę, która z wartości będzie tylko do odczytu po utworzeniu obiektu?

Być może po skonstruowaniu można by przypisać właściwości do publicznego zapisu.

Skąd ostatecznie pochodzą wartości? Być może niektóre wartości są naprawdę zewnętrzne, podczas gdy inne pochodzą z jakiejś konfiguracji lub danych globalnych utrzymywanych przez bibliotekę.

W takim przypadku możesz ukryć konstruktor przed zewnętrznym użyciem i udostępnić mu funkcję Create. Funkcja create pobiera prawdziwie zewnętrzne wartości i konstruuje obiekt, a następnie używa akcesorów dostępnych tylko w bibliotece do ukończenia tworzenia obiektu.

Byłoby naprawdę dziwne, gdyby obiekt wymagał 7 lub więcej parametrów, aby nadać obiektowi kompletny stan, a wszystkie są z natury zewnętrzne.

Gdy klasa ma konstruktora, który przyjmuje zbyt wiele argumentów, zwykle jest to znak, że ma zbyt wiele obowiązków. Prawdopodobnie można go podzielić na osobne klasy, które współpracują ze sobą, aby nadać te same funkcje.

W przypadku, gdy naprawdę potrzebujesz tak wielu argumentów do konstruktora, wzorzec Builder może ci pomóc. Celem jest nadal przekazywanie wszystkich argumentów do konstruktora, więc jego stan jest inicjowany od samego początku i nadal można uczynić klasę niezmienną w razie potrzeby.

Zobacz poniżej:

public class Toto {
    private final String state0;
    private final String state1;
    private final String state2;
    private final String state3;

    public Toto(String arg0, String arg1, String arg2, String arg3) {
        this.state0 = arg0;
        this.state1 = arg1;
        this.state2 = arg2;
        this.state3 = arg3;
    }

    public static class TotoBuilder {
        private String arg0;
        private String arg1;
        private String arg2;
        private String arg3;

        public TotoBuilder addArg0(String arg) {
            this.arg0 = arg;
            return this;
        }
        public TotoBuilder addArg1(String arg) {
            this.arg1 = arg;
            return this;
        }
        public TotoBuilder addArg2(String arg) {
            this.arg2 = arg;
            return this;
        }
        public TotoBuilder addArg3(String arg) {
            this.arg3 = arg;
            return this;
        }

        public Toto newInstance() {
            // maybe add some validation ...
            return new Toto(this.arg0, this.arg1, this.arg2, this.arg3);
        }
    }

    public static void main(String[] args) {
        Toto toto = new TotoBuilder()
            .addArg0("0")
            .addArg1("1")
            .addArg2("2")
            .addArg3("3")
            .newInstance();
    }

}