Uniknięcie bloku static w każdej z samo rejestrujących się klas

0

Hej!

Czy jest jakaś możliwość stworzyć pewną grupę klas implementujących dany interfejs, w której każda z tych klas zgłosi swoje istnienie do fabryki bez użycia zewnętrznych bibliotek i bloku static?

Mam sobie interfejs

public interface Order {
    void execute();
}

i chciałbym, żeby każda klasa zgłosiła się do mojej fabryki

public class OrdersFactory {

    private static HashMap<String, Class<? extends Order>> orders = new HashMap<>();

    public static void register(String command, Class<? extends Order> orderClass) {
        if (command == null || orders.containsKey(command))
            throw new IllegalStateException("Command is null or already exists");
        else if (orderClass == null)
            throw new IllegalStateException("Class to be registered cannot be null");
        orders.put(command, orderClass);
    }

    public static Order getOrderInstance(String command) {
        if (!orders.containsKey(command))
            return new InvalidOrder();
        return createInstance(orders.get(command));
    }

    private static Order createInstance(Class<? extends Order> orderClass) {
        try {
            return orderClass.newInstance();
        } catch (InstantiationException e) {
            System.out.println("An error occurred while creating command class instance");
        } catch (IllegalAccessException e) {}
        return new InvalidOrder();
    }

    public static List<String> findPossibleValidCommands(String invalidCommand) {
        return orders.keySet()
                .stream()
                .filter(command -> command.contains(invalidCommand))
                .collect(Collectors.toList());
    }

}

Przykładowo wygląda to tak

public class HelloOrder implements Order {

    @Override
    public void execute() {
        System.out.println("Hello");
    }

    static {
        OrdersFactory.register("hello", HelloOrder.class);
    }
}

Ale moim zdaniem jest brzydkie, bo ciągle muszę robić CTRL + C, CTRL + V, a wtedy tracę smak życia.
Jakieś pomysły?

2

To jeden z powodów powstania springa ;).

Na początku musisz ogarnąć co robisz źle. Po pierwsze chcesz zrobić signleton w stylu obiektu scali/kotlina, ale jednocześnie nie chcesz używać staticów. To jest główny problem. Musisz się zdecydować czy robisz czysty obiekt wielu istancji, czy singleton. Osobiście sądząc po tym co chcesz uzyskać zrobiłbym singleton i ręcznie w konstruktorze go inicjował (tak po bożemu ;)).

Dodatkowo brakuje OrderFactory (bez "s"), który byłby przypisany do konkretnego orderu i wiedziałby jak stworzyć dany Order bez runtimeowej magi.


interface OrderFactory<T extends Order>{
   String orderCode();
   T createOrderInstance();
}

class SomeOrderFactory<SomeOrder>{
    String orderCode() {return "SOME_ORDER";}
    SomeOrder createOrderInstance(){ new SomeOrder();}
}

class OrdersFactory {
    private static OrdersFactory instance = new OrderFactory(new SomeOrderFactory());
    private final HashMap<String, OrderFactory> orders;
   
   OrdersFactory(Collection<OrderFactory> orderFactories){
        orders = orderFactories.stream().collect(Collectors.toMap(OrderFactory::orderCode, Function::identity());
   }

    public Order getOrderInstance(String command) {
        Optional.ofNullable(orders.get(command))
             .map(OrderFactory::createOrderInstance)
             .orElseThrow(InvalidOrder::new);
    }
}

...


OrdersFactory.instance.getOrderInstance("SOME_ORDER"); // Zwraca SomeOrder

Pisane z palucha więc pewnie są literówki.
BTW. 2 baty chłosty za nieobsłużony exception

0

Okej, mały przegląd tego co właśnie zrobiłem i stanąłem w jednym miejscu.
Mam:

public interface Order {
    void execute();
}

oraz przykładowy

public class HelloOrder implements Order {

    @Override
    public void execute() {
        System.out.println("Hello");
    }
}

Do tego mam:

public interface OrderFactory<T extends Order> {
    String orderCommand();
    T createOrderInstance();
}

oraz zgodnie z przykładem

public class HelloOrderFactory implements OrderFactory<HelloOrder> {

    @Override
    public String orderCommand() {
        return "hello";
    }

    @Override
    public HelloOrder createOrderInstance() {
        return new HelloOrder();
    }
}

Tu dla mnie wszystko jest raczej jasne - każdy Order ma swoją fabrykę.
Problemy zaczynają się w OrdersFactory (ostatniej klasie z posta wyżej)

public class OrdersFactory {

    private static OrdersFactory instance = new OrdersFactory(); // To ma być sigleton
    private final HashMap<String, ? extends OrderFactory> orders = new HashMap<>(); /* Ta mapa ma zawierać
	wszystkie fabryki (czyli klasy implementujące OrderFactory) i pasujące do nich komendy */

	// Tu jest problem :/
    OrdersFactory(Collection<OrderFactory> orderFactories){
        orders = orderFactories.stream().collect(Collectors.toMap(OrderFactory::orderCommand, Function::identity());
    }

    public Order getOrderInstance(String command) {
        return Optional.ofNullable(orders.get(command))
                .map(OrderFactory::createOrderInstance)
                .orElse(new InvalidOrderFactory().createOrderInstance());
    }
}

Za nic nie mogę się domyśleć skąd wziąć kolekcję wszystkich fabryk...

1

Widzę 2 opcje.

  1. Sam uzupełniasz w OrdersFactory
  2. Używasz magii w stylu Spring/Guice czy inny CDI (ale nie robisz tego sam).

Rozwiązanie pierwsze jest wbrew pozorom całkiem niezłe. Wprawdzie musisz dodać jedną linijkę gdy dołączasz nowy Order, ale wtedy tak czy siak musisz napisać z 50 w samych klasach Order i OrderFactory nie licząc testów. Tak więc ból jest mały, a za to dostajesz czytelność i minimalny stopień skomplikowania. Problem pojawia się gdybyś chciał robić coś w stylu pluginów, czyli np. dołączasz jar w trakcie działania aplikacji. Wtedy oczywiście dochodzi runtime, ale jest to bardzo odosobniony przypadek.

Generalnie wiele aplikacji Java cierpi na przeinżynierowanie. Ktoś wymyśli jakiś magiczny sposób na zrobienie banalnej rzeczy i na początku podjarka jakie to fajne, a po pół roku nikt nie wie jak to do końca działa, autora w firmie nie ma i strach ruszać żeby się całość nie rozleciała. Dlatego jeżeli jest to przypadek który należy do 99% problemów aplikacji statycznych i zmienianych tylko łącząc z rekompilacją to nie ma co szukać super hiper rozwiązań na siłę.

Widzę też że zwracasz błędny obiekt przy braku odpowiedniej fabryki. Ja napisałem źle, ale miało być .orElseThrow(InvalidOrderException::new, czyli rzucenie wyjątku. Jeżeli jest to aplikacja statyczna, to nie ma sensu rozwodzić się nad brakującym elementem. To ewidentna wina programisty że masz nieobsłużony przypadek, więc najlepiej niech ten wyjątek wyleci gdzieś wysoko, sytuacja zostanie zalogowana a apka ubita. Masz wtedy pewność że ktoś ten prosty problem szybko wychwyci i naprawi. Nie staraj się naprawić problemów których nie ma sensu naprawiać połowicznie.

0

Widzę też że zwracasz błędny obiekt przy braku odpowiedniej fabryki. Ja napisałem źle, ale miało być .orElseThrow(InvalidOrderException::new, czyli rzucenie wyjątku. Jeżeli jest to aplikacja statyczna, to nie ma sensu rozwodzić się nad brakującym elementem. To ewidentna wina programisty że masz nieobsłużony przypadek, więc najlepiej niech ten wyjątek wyleci gdzieś wysoko, sytuacja zostanie zalogowana a apka ubita. Masz wtedy pewność że ktoś ten prosty problem szybko wychwyci i naprawi. Nie staraj się naprawić problemów których nie ma sensu naprawiać połowicznie.

Myślałem o tym w innym kontekście. Program będzie konsolowy (na razie), a pomyłki przy wklepywaniu do terminalu to chleb powszedni (sam widzę ile razy się mylę na moim ubunciaku) i stwierdziłem, że wyjątek to za dużo. Mogę stworzyć Order, który w razie wklepania złej komendy mnie o tym poinformuje (i poda komendy, które mogłem mieć na myśli, nad tym jeszcze będę pracował). Z zasady każdy Order ma być osobnym podprogramem niezależnym od reszty, więc nic nie stracę, a jednocześnie nie obciążam systemu 15 wyjątkami w ciągu minuty. Czy to dobre podejście?

Czy taka rejestracja klas będzie poprawna?

    private OrdersFactory(){
        ArrayList<OrderFactory> allFactories = new ArrayList<OrderFactory>() {{
            add(new HelloOrderFactory());
            add(new InvalidOrderFactory());
            // etc
        }};
        allFactories.forEach(orderFactory -> orders.put(orderFactory.orderCommand(), orderFactory));
    }

I tak swoją drogą jestem pod wrażeniem doświadczenia. Problem nad którym ja siedzę dobę, Ty rozwiązujesz w 10 minut, od niechcenia ;)

1

Na początek, własna implementacja ArrayList w tym wypadku to trochę zbyt dużo ;). Polecam:
https://docs.oracle.com/javase/7/docs/api/java/util/Arrays.html#asList(T...)

Druga sprawa, to warto zostawić konstruktor który przyjmuje kolekcję, ponieważ przyda się on do tworzenia testowalnej fabryki. Do testów jednostkowych nie potrzebujesz przecież tysiąca komend, tylko jednej, która tak naprawdę nie ma znaczenia więc może być to klasa w testach. W bezparametrowym konstruktorze natomiast można tworzyć to czego chcesz użyć domyślnie.

...
    private static OrdersFactory instance = new OrdersFactory(); // To ma być sigleton

  OrdersFactory(Collection<OrderFactory> orderFactories){
        orders = orderFactories.stream().collect(Collectors.toMap(OrderFactory::orderCommand, Function::identity());
    }

    private OrdersFactory(){
       this(Arrays.asList(
            new HelloOrderFactory(),
            new InvalidOrderFactory(),
       ));
    }
...

Działa podobnie jak powyżej, z tym że w testach możesz zrobić:

OrdersFactory testedOrdersFactory = new OrdersFactory(new ForTestsOnlyOrderFactory());

@Edit
Jeżeli chcesz użyć tego tak jak napisałeś do konsoli to jest ok z tym InvalidOrder. Myślałem że to zamknięty przypadek.

0

To nie własna implementacja ;) tylko ciekawostka w Javie :P

  OrdersFactory(Collection<OrderFactory> orderFactories){
        orders = orderFactories.stream().collect(Collectors.toMap(OrderFactory::orderCommand, Function::identity());
    }

Tu jest problem z tym, że OrderFactory::orderCommand nie jest statyczne, Function::identity() nie istnieje, ale popracuję nad tym później. Wielkie dzięki za dużo dawkę nowej wiedzy ;)

0
Burdzi0 napisał(a):

Hej!

Czy jest jakaś możliwość stworzyć pewną grupę klas implementujących dany interfejs, w której każda z tych klas zgłosi swoje istnienie do fabryki **bez użycia zewnętrznych bibliotek i bloku static
..
Ale moim zdaniem jest brzydkie, bo ciągle muszę robić CTRL + C, CTRL + V, a wtedy tracę smak życia.
Jakieś pomysły?

Zaproponuję inne podejście - niech fabryka sobie przeskanuje i sama znajdzie to co jest potrzebne. Przyjrzyj się pakietowi reflection.

1

@Burdzi0 brniesz w jakieś straszne dziwy. Użycie reflection to dobry znak, że się własnie coś bardzo zepsuło.
Pytanie zasadnicze - co Ty chcesz właściwie zrobić? Jaki jest cel tej zabawy?

EDIT: kumam co Ty chyba robisz (opis był w środku kolejnego posta.)

Czemu nie tak?

OrdersFactory meineKleineFactory = OrdersFactory .empty()
 .register ( new HelloOrderFactory())
 .register( new FormatDiskFactory())
  .register( new StartQuake2Factory())
   ...

    

I może wtedy nie musisz mieć singletona ? Bo to chyba źródło twojego problemu.

0

@jarekr000000: Chciałem stworzyć mini bota (który im dłużej się zastanawiam tym bardziej się utwierdzam w przekonaniu, że z botem nie ma nic wspólnego), który przyjmowałby komendę, a następnie wykonywał podprogram z nią związaną (czyli chcę linuxowy terminal, what a brilliant idea). Chciałem stworzyć samo rejestrujące się klasy. @krzysiek050 stworzył dla mnie strukturę, która pozwala na bardzo łatwe budowanie (np. niezależnie od tego, co jest wymagane w konstruktorze) poprzez fabryki. Teraz chciałbym załatwić rejestrowanie klas. @wartek01 proponuje refleksję. Ja się zastanawiam osobiście czy adnotacja rozwiązałaby ten problem - jeśli jest adnotacja to dodaj fabrykę do listy. Twój pomysł rzeczywiście trochę ułatwi, ale w mojej głowie ciągle biega - czy da się to zrobić tak, żeby samo się zrobiło :P Ręczne dodawanie jest ok, ale rozszerzalnie projektu obejmuje wtedy grzebanie tam, gdzie niekoniecznie chciałbym mieszać. Jeśli rozwiązanie, które otrzymałem od @krzysiek050 jest wystarczającym ułatwieniem to mnie przystopujcie ;)

0

Zastanowiłbym się jaką funkcjonalność chesz dokładnie uzyskać.Jeśli chcesz program rozdawać dla mas i ludzie będąmogli dodawać własne komendy do katalogu plugins, w postaci jarów z kodem to refleksja ma sens.
Jesli jednak robisz to dla siebie, to w zasadzie nieważne czy dodasz kolejną linijkę gdzieś w registry czy wrzucisz klasę wychodzi na to samo funkcjonalnie. Użycie refleksji jednoczesnie utrudni Ci w przyszłości jakieś refaktoringi i będziesz miał zdecydowanie bardziej błedogenny kod.
Chyba, że po prostu chcesz sie pobawić refleksją :-) To zabawny i mocny mechanizm, szkoda, że nadużywany przez tony frameworków.

0

Samorejestrujące się klasy to krok w złą stronę, bo utrudnia śledzenie (czytanie i rozumienie) kodu. Magia (jeśli już ktoś się na nią uprze) powinna być dobrze udokumentowana i rozpoznawalna (jak np kontenery do wstrzykiwania zależności czy ORMy). W innym przypadku kod stanie się zaskakujący, a to prowadzi do błędów w rozwoju programu. Niewidzialna ręka refleksji robi coś za naszymi plecami, a my domyślamy się co może się dziać. Jeśli nasza intuicja jest zła (a zwykle jest zła - życie...) to będziemy w zły sposób rozwijać kod.

Jeśli chodzi o automatyczne ładowanie wtyczek to ZTCP IntelliJ stosuje osobny classloader dla każdej wtyczki, po to by uniknąć dependency hell. Poszczególne wtyczki z zewnątrz mogą mieć zależności w różnych wersjach (np jedna wtyczka korzysta z JanuszLIB 1.0, a druga wtyczka z JanuszLIB 2.0), a my nie chcemy by coś się posypało. Dlatego classloader dla wtyczki widzi classpath dla IntelliJa oraz ma dostęp do bibliotek związanych z tą jedną konkretną wtyczką, ale nie z innymi wtyczkami. Kolejna sprawa z wtyczkami to to, że wtyczka może być wadliwa. Wtedy pasuje ją wyłączyć. IntelliJ ma taki mechanizm. Ogólnie mechanizm wtyczek w IntelliJu polega na szukaniu JARów w określonym katalogu, sprawdzeniu czy w rejestrze IntelliJowym wtyczka jest włączona i jeśli tak to ładowaniu jej do osobnego classloadera. Jeśli chcesz mieć porządny mechanizm wtyczek to musisz zrobić coś podobnego.

I jeszcze jedna sprawa: bloki statyczne są uruchamiane przy ładowaniu klasy, a ładowanie klas w Javie jest leniwe (wyjaśnienie np tutaj: http://javarevisited.blogspot.com/2012/07/when-class-loading-initialization-java-example.html ). Wstawienie bloku static do klasy nie załatwia sprawy, bo trzeba wymusić załadowanie tej klasy, by blok static się wykonał. Zamiast tak kombinować można po prostu zarejestrować wszystkie potrzebne klasy w jednym miejscu.

0
Wibowit napisał(a):

I jeszcze jedna sprawa: bloki statyczne są uruchamiane przy ładowaniu klasy, a ładowanie klas w Javie jest leniwe (wyjaśnienie np tutaj: http://javarevisited.blogspot.com/2012/07/when-class-loading-initialization-java-example.html ). Wstawienie bloku static do klasy nie załatwia sprawy, bo trzeba wymusić załadowanie tej klasy, by blok static się wykonał. Zamiast tak kombinować można po prostu zarejestrować wszystkie potrzebne klasy w jednym miejscu.

Właśnie miałem to pisać. Gdyby robić takie auto rejestrujące się klasy, to albo refleksje (i to chyba dość magiczne) albo napisać własny annotation procesor, który zbierze tak oznaczone klasy, wygeneruje z nich metodę zbierającą oznaczone klasy w listę i rejestrującą je w factory. Tylko szczerze, to chyba lepiej napisać tę listę z ręki.

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