Wiele parametrów konstruktora czy klasa abstrakcyjna?

0

Refaktoruje ostatnio bardzo dużo słabego kodu w pracy i w trakcie usuwania duplikacji zacząłem się zastanawiać nad pewną kwestią. Załóżmy, że wydzielam sobie kod pewnego panelu GUI. Panel jest używany w wielu miejscach i jest generalnie identyczny, ale różni się:

  • labelkami
  • tooltipami
  • akcjami jak ktoś cośtam kliknie na przykład

Gdyby tych parametrów było mało i gdyby nie było tam "akcji" to pewnie bez zastanowienia wrzucałbym je przez konstruktor. Liczbę parametrów zawsze można ograniczyć jakimś DTO ale to taki sobie pomysł bo to DTO nigdzie nie byłoby używane a jedynie tworzone w miejscu tworzenia panelu z parametrów znanych w chwili kompilacji. Więc dość słabo. Dodatkowo "akcje" trzeba by wrzucać jako jakieś lambdy albo funktory.
Dodatkowo część tych parametrów może być zostawiona jako domyślna, co mocno komplikuje opcje z konstruktorem, bo albo mamy mother of all constructors a potem kupę nulli jako parametry albo mamy milion konstruktorów (mało możliwe bo niektóre parametry są tego samego typu ;) ).

W związku z tym pomyślałem że można to zrobić z zupełnie innej strony. Zrobić klasę abstrakcyjną z metodami w stylu getXYZTooltip() a wymagane akcje zrobić jako metody abstrakcyjne. Wtedy tworząc obiekt musimy zaimplementować wymagane metody i ewentualnie przeładować te gdzie nie chcemy używać opcji domyślnych. Tylko że to rozwiązanie wydaje mi się trochę dziwne, bo jednak tworzymy nową klasę (choćby i anonimową) podczas gdy w rzeczywistości te klasy różnią się właściwie tylko parametrami (szczególnie jeśli uznamy lambdy za parametry).

Co o tym myślicie?
@somekind @Koziołek @Krolik @niezdecydowany @karolinaa @msm @winerfresh @stryku @Patryk27 @katelx @WhiteLightning @datdata @spartanPAGE @Wizzie @Azarien @Satirev @MarekR22 @Afish @panryz @Wibowit @azalut i więcej mi się nie chce wymieniać, ale niech się nikt nie czuje pominięty! (kolejność losowa!)

9

Pokaż kod :P

4

Może po prostu builder?

1

w tym panelu się layout zmienia? czy wszystko zostaje tak samo tylko "napisowe elementy" sie zmieniają?

w panelu zmieniają się też np buttony? bo jak przeczytałem to pomyslalem, że jeśli występowałby taki związek między tymi elementami, że np: jest buton, każdy buton ma label, kazdy label ma tooltip i jeszcze action na kliknięcie to możnaby stworzyć obiekt upakowujący te pare obiektów i takie zestawy dodawać przez konstruktor i zrobić jakiś limit (nie wiem czy to ta sama idea o której mówisz pisząc o DTO). Ale to wtedy jeśli cały layout albo chociaż takie "paczki" obiektów sie zmieniaja na tym twoim panelu

Co do twojego pomysłu, mówisz tylko o akcjach na kliknięcie? czy chodzi o taką defaultową klase dla panelu/elementów gui i ew. reimplementacji tego co akurat potrzeba? (tzn np getXYZTooltip() zmieniasz napis tipa) bo nie do końca łapie

3

Wujek Bloch faktycznie, powie Builder :) ale ciężko w ogóle powiedzieć, bo jeżeli masz już zdefiniowane że ta klasa ma 5 tooltipów (

getXYZTooltip() 

) i one są zawsze wymagane, to raczej ciężko zrobić z tego "pikny" kod :D Zamiast tej klasy abstrakcyjnej, ciała tych metod (które wcześniej chciałeś po prostu nadpisać) lepiej chyba podać lambdą - nie musisz za każdym razem robić nowej anonimowej klasy (+10000 do wydajności :)) a i lepiej wygląda (i te akcje mogą być faktycznie podawane builderem)


setXXTooltip(p->p.costamZrob()).setYYTooltip(p->p.sendWiadomoscDoGdziestam).setZZZTooltip(p::costam);

A tak serio to napisałem tylko po to żeby poczuć się inteligientny D:D w końcu umiem tylko klepać formy DDD

0

@mychal builder to faktycznie jest klasyczne rozwiązanie jeśli chodzi o problem z konstruktorem, niemniej nadal będę miał gigantyczną "wiązankę" tworzącą taki panel. A mam okienko gdzie jest ich kilka obok siebie ;) Rozwiązanie z klasą abstrakcyjną pozwala wyrzucić ten kod do osobnej klasy i na oko wygląda trochę czytelniej.

@Wibowit @datdata akurat kod nie ma tu specjalnie wiele do rzeczy. Chodzi mi raczej o taką teoretyczną rozkminę ;)

@azalut w panelu nie zmieniają się komponenty, tzn masz tam jakieś pole tekstowe, jakieś buttony etc i to jest stałe. Zmieniają się labelki, tooltipy i akcje. Jeśli chodzi o to nadpisywanie to wyobraź sobie że np. metoda getTooltipForTextFieldX() defaultowo zwraca pustego stringa a jak w danym miejscu chcesz mieć tooltip to ja przeładowujesz. Trochę tak jak np. javowe klasy XYZAdapter dla ActionListenerów.

2

A są jakieś powielające się zbiory własności? Jeśli tak, to możesz zrobić metodę klonującą w budowniczym oraz metody dodające zachowania, np metoda typu:

PanelBuilder addYellowTooltips() {
  tooltipsColor = Yellow;
  return this;
}

Później możesz mieć konstrukcje tego typu:

PanelBuilder genericBuilder = new PanelBuilder().setA(blabla).setB(bleble);
PanelBuilder passwordBuilder = genericBuilder.clone().setTitle("Password reset").showLabel(Labels.PasswordLabel);

Jeśli chodzi o dowolne składanie akcji to możesz zrobić interfejs ze wszystkimi wspólnymi akcjami. Do tego klasę, która ma puste lub domyślne implementacje każdej akcji. Następnie możesz użyć wzorca https://pl.wikipedia.org/wiki/%C5%81a%C5%84cuch_zobowi%C4%85za%C5%84_(wzorzec_projektowy) (chain of responsibility) do zaimplementowania składania zachowań. Kod mógłby być taki:

interface Actions {
  void actionA();
  void actionB();
  void actionC();
}

class ActionsAdapter {
  void actionA() {}
  void actionB() {}
  void actionC() {}
}

class ActionAdapterChain extends ActionsAdapter {
  final ActionsAdapter previous;
  // konstruktor

 // metody delegujące do previous
}

class PanelBuilder {
  Actions actions = new ActionsAdapter();
  ...
  PanelBuilder blinkOnClick() {
    Actions oldActions = actions;
    actions = new ActionAdapterChain(actions) {
      @Override void actionA() { 
        oldActions.actionA(); // albo można olać tę linijkę
        blink(); 
      }
    }
    return this;
  }
  ...
  PanelBuilder genericBuilder = ...
  PanelBuilder blinkingBuilder = genericBuilder.clone().blinkOnClick();
}
0

@Shalom już chyba rozumiem, wydaje się dobre ale ja to widze jakos tak:
zrobić zwykłą klasę, nie abstrakcyjna i defaulotowo zrobić tak jak mowisz, na przyklad.. hm.. mieć pare pól z defaultowymi akcjami. Dla tipa bedzie to pusty String, a dla akcji na klikniecie bedzie do jakis @FunctionalInterface
I teraz - zrobisz obiekt panelu i wszystko jest defaultowo, a jesli chcesz zmienic zachowanie np dla klikniecia butonu to robisz dla każdego pola setter i wstawiasz nowego stringa albo nową funkcję

(jesli dobrze cie rozumiem) to wyprodukowaloby troche mniej kodu niz abstrakcyjna anonimowa klasa i implementowanie tego co chcesz, szczegolnie, ze lambdy mozesz tam wrzucic a to "jednolinijkowce"
Jesli takich nie-defaultowych wymagan byloby nie duzo to kod tez nie bylby jakis rozwlekly
ma to sens jakiś? bo to nagle mi przyszlo do glowy :D

0

Jeszcze tego w taki sposób nie robiłem, ale czy nie jako pomoc w tego typu sytuacjach nie powstało to:

https://docs.oracle.com/javase/tutorial/java/IandI/defaultmethods.html

0

@Wibowit właściwości się nie powtarzają w zasadzie nigdy. Akcje zresztą też nie, bo "akcje" to są zwykle wywołania prezentera/kontrolera. Może krótki opis o jaką sytuację mi chodzi:

Założmy że mam sobie panel który ma pole tekstowe, guzik który robi "browse" i pozwala wybrać plik z dysku oraz guzik "process". Są tam jeszcze jakieśtam labelki, tooltipy itd. I teraz identyczny panel jest używany w kilku miejscach w aplikacji, ale do zupełnie innych "celów" z punktu widzenia biznesowego. Tzn dotyczy zupełnie innego widoku a "process" wywołuje zupełnie inne akcje kontrolera. Więc robienie listy wszystkich akcji albo takiego generycznego buildera odpada.

Zwykły builder jak najbardziej by sie nadał, ale jego słaba strona to fakt że jak mam widok gdzie jest takich paneli np. 5 to nagle się tam robi 100 linii kodu tworzenia tych paneli z odpowiednich parametrów ;]

@azalut jasne że można i tak, ale to sie niczym nie rożni od buildera ;) Kodu technicznie byłoby mniej, ale byłby upchany w jednym miejscu ;)

0

Jak już taka rozkminka to może FluentAPI? Domyślnie masz jeden domyślny konstruktor, który ustawia wszystko na wartość zerową/pustą, następnie piszesz sobie jakieś extension functions, żeby działało to tak:

Potrzebujesz okienko z jednym tooltipem i jedną akcją:

      BaseWindow.Create().WithTooltip("id", "text").WithButton("buttonID").WithAction("componenID", () => { /* ... */ });

Żeby nie trzeba było powtarzać kodu do tworzenia tych samych "komponentów" w różnych okienkach, może i coś takiego:

      var sharedComponents =  ComponentsCollection.Create().WithTooltip("id", "text").WithButton("buttonID").WithAction("componenID", () => { /* ... */ });
      var customWindow1 = BaseWindow.Create().WithComponents(sharedComponents).WithButton("buttonID2").WithAction("buttonID2", () => { /* ... */});
      var customWindow2 = BaseWindow.Create().WithComponents(sharedComponents).WithTooltip("ID", "TEXT2");

Nie wiem czy to dobry pomysł :D

0

Jaki framework? Takie rzeczy powinny/mogą być rozwiązane już na poziomie użytego framework (nie ma nic gorszego jak usilne walczenie z frameworkiem).
Przykładowo w Qt jest QAction, które robi dokładnie to co potrzebujesz.
Do jednego UI podłącza się QAction, które dostarcza tytuł ikonkę i zachowanie, a w razie potrzeby stan UI (czy element powinien być checkable, enabled itp).

0

Generalnie tak:
ja jakimś mega JAVAMASTER nie jestem, aale pierwsze o czym pomyślałem to wspomniany Builder

0

oddaję głos na klasę abstrakcyjną. builder w javie to trochę boilercode taki a lombok i te step builder generatory pluginy nawet nie umieją wygenerować step buildera z opcjonalnymi polami. parametry przekazywane przez klasę anonimową do dużego konstruktora też są dla mnie spoko. zajmują sporo linii ( @Override ) , ale chociaż wszystko widać jak na dłoni.

0

Jak widzę nie mogę chodzić spać o ludzkich porach, bo pomijam ciekawe dyskusje :)

Miałem podobny problem w jednym z projektów tyle tylko, że zamiast przepisywać coś co już istnieje rzeźbiłem od zera. Problem podzieliłem na dwa obszary:

  1. tooltipy i labelki - to jest tak naprawdę jeden i ten sam problem pobierania danych, ale w różnych ujęciach.
  2. akcje - ten problem jest "bliżej" kodu.

Problem 1. można łatwo rozwiązać poprzez użycie "magii" w postaci (można to zrobić ładniej) labelService.getLabel(getClass().getCanonicalName() + ".myLabel1") i następnie w pliku properties masz litanię opisów. Wystarczy wstrzyknąć serwis. Zaleta - jest proste (prościej się nie da), wada - jeżeli w ramach jednej klasy masz dwa obiekty z różnymi napisami to musisz jeszcze rozszerzyć o jakiś "name" obiektu. Całość można ogarnąć jak napisał @azalut za pomocą interfejsów i default metod, tak by w punkcie gdzie używasz wystarczyło napisać getLabel("myLabel") i gotowe.

Problem 2. rozwiązałem przez klasę abstrakcyjną, która dostarczała szablon GUI oraz metody abstrakcyjne do tworzenia akcji. Zaletą jest znowuż prostota i duża intuicyjność rozwiązania. Wadą, chyba najpoważniejszą, jest dość trudna implementacja dla dynamicznych struktur typu tabele o zmiennej ilości kolumn. Trzeba wtedy nadpisywać część zachowania bądź tworzyć metody abstrakcyjne, które będą dostarczały całe bloki GUI.

0

A może połączenie buildera i protypu? Najpierw inicjujujesz builder jakimś już zbudowanym panelem, a potem tylko budujesz nadpisując właściwości prototypu. Nie robiłbym też specjalnego rozróżniania na akcje/tooltipy/buttony. W większości języków przekazujemy je do metod tak samo i z punktu widzenia budowania jest tym samym, czyli właściwością panelu.

0

@Koziołek
ad.1. Tak jak pisałem wcześniej, te labelki i tooltipy się nie powtarzają w zasadzie więc taki generyczny serwis który by je serwował wydaje się taki-sobie. Szczególnie że nie muszę sie tu martwić o jakąś lokalizację czy internacjonalizację - w takiej sytuacji musiałbym tak czy siak eksternalizować te teksty i ładować dynamicznie i wtedy opcja z pobieraniem na podstawie ID miałaby sens.
ad.2. Tutaj akurat problem nie jest aż tak złożony bo akurat to są proste komponenty.

0

@Shalom, eksternalizuj stringi gdzie się tylko da. Nie ma nic gorszego niż litrówka, której poprawienie wymaga rebuildu całej apki ;) cała magia tego serwisu polega na tym, że ostro oddzielasz dane (treść napisów) od apki.

0

Koziołek ma racje- co się da wywalić do plików konfiguracyjnych. Nie znam szczegolow, ile tam jest tych akcji, jako dokladnie wyglada, ale mozna tez zrobic w apce aby dalo sie wybierac akcje na podstawie configu (taki actionSelector).

1

We wzorcu builder zrobiłbym metody tworzące zdefiniowane z góry ogólne panele, które potem klonowałbym i rozszerzał o pozostałe elementy. Składanie każdego panelu przez wywołanie wszystkich metod jest słabe, bo co zrobisz, gdy trzeba będzie zmodyfikować jakąś głupotę we wszystkich panelach? Ogólnie rozwiązanie @Wibowit wydaje się całkiem niezłe na początek, ale bez znajomości całego kodu jest to wróżenie z fusów.
@Koziołek dobrze prawi o plikach konfiguracyjnych z etykietami. Na początku wystarczy hardkodowanie identyfikatorów, potem można pomyśleć nad jakąś sprytną refleksją.
Jeżeli chodzi o akcje, to zależy od ich wielkości - jeżeli jest to jedna linijka, to lambda przejdzie, ale jak ma być to coś grubszego, to wzorzec strategia + jakiś pipelining. Jeżeli akcje też są podobne, to można i tutaj użyć buildera.

0

@Koziołek @WhiteLightning ja nie mówię że to jest zły pomysł, ale wolę robić takie zmiany inkrementalnie, tzn najpierw podzielić cały ten kod po ludzku na MVP, potem usunąć duplikacje i powydzielać wspólne elementy, potem zrobić jakiś strukturalny refaktoring (podzielić wielkie klocki na małe metody, powydzielać klasy etc) a dopiero potem się brać za takie szczegóły jak wyrzucenie stringów do osobnej konfiguracji ;)

2

@Shalom! ktos ci sie na konto wlamal i pytania zadaje ;)

ja bym szla w kierunku tych klas abstrakcyjnych, tak zeby akcje byly metodami a caly smietnik zwiazany z labelami, tooltipami etc upakowac w cos slownikopodobnego (skoro nie chcesz miec zewnetrznych zasobow), tzn idkontrolki:wartosc i trzymac jako jednego gettera.
oczywiscie wszystko opakowac w jakas fabryke, w koncu java ;)

1 użytkowników online, w tym zalogowanych: 0, gości: 1