Jak byście taki śmietnik uporządkowali ?

0

Witam, jestem początkującym programistą Java. Obecnie zastanawiam się jak by uporządkować ten kod, czy podzielić je na klasy abstrakcyjne, czy interfejsy, czy dodać więcej metod. Jakie są Wasze sposoby na uporządkowanie takiego kodu ?

 private static String runMight(TypeCaptcha typeCaptcha, String apiKey, String filePath, String urlWebsite, String keyWebsite, Integer attempts, Integer timeoutAttempt, Integer might, String userAgent, ProxyTypeOption proxyTypeOption, String proxyAddress, Integer proxyPort, String proxyLogin, String proxyPassword) throws AntiCaptchaException {
        String result = null;
        try {
            SimpleObjectProperty<Throwable> error = new SimpleObjectProperty<>();
            SimpleStringProperty solution = new SimpleStringProperty(null);
            Integer countSyncAttempt = 0;
            while (countSyncAttempt < attempts) {
                countSyncAttempt++;
                ExecutorService executorService = AsyncHelper.async(() -> {
                    List<Callable<Object>> callables = new ArrayList<>();
                    for (int i = 0; i < might; i++) {
                        final Integer count = i;
                        callables.add(() -> {
                            String s = null;
                            switch (typeCaptcha) {
                                case IMAGE_TO_TEXT:
                                    s = AnticaptchaHelper.imageToText(apiKey, filePath);
                                    break;
                                case RECAPTCHA_V2:
                                    s = AnticaptchaHelper.noCaptchaProxyless(apiKey, urlWebsite, keyWebsite);
                                    break;
                                case RECAPTCHA_V2_PROXY:
                                    s = AnticaptchaHelper.noCaptcha(apiKey, urlWebsite, keyWebsite, userAgent, proxyTypeOption, proxyAddress, proxyLogin, proxyPassword, proxyPort);
                                    break;
                                default:
                                    s = AnticaptchaHelper.imageToText(apiKey, filePath);
                            }
                            if (Objects.isNull(solution.get())) {
                                synchronized (solution) {
                                    solution.set(s);
                                }
                            }
                            return null;
                        });
                    }
                    try {
                        AsyncHelper.asyncMultiWait(callables, callables.size(), "Anticaptcha", true, Thread.NORM_PRIORITY);
                    } catch (AsyncHelperException e) {
                        error.set(e);
                    }
                }, "Anticaptcha Main", false, Thread.NORM_PRIORITY);
                Integer timeoutAttemptCountMilliseconds = 0;
                Integer poolingEvery = 100;
                while (true) {
                    if (Objects.nonNull(solution.get()))
                        break;
                    TimeUnit.MILLISECONDS.sleep(poolingEvery);
                    timeoutAttemptCountMilliseconds += poolingEvery;
                    if (timeoutAttemptCountMilliseconds >= timeoutAttempt * 1000) {
                        break;
                    }
                }
                if (Objects.nonNull(solution.get())) {
                    executorService.awaitTermination(1, TimeUnit.NANOSECONDS);
                    break;
                }
            }
            if (Objects.nonNull(error.get())) {
                throw new AntiCaptchaException(error.get());
            }
            result = solution.get();
        } catch (Exception e) {
            throw new AntiCaptchaException(e);
        }
        return result;
 
1
  1. Podziel to na metody w taki sposób, żeby każda robiła tylko jedną rzecz
  2. Jeżeli okaże się że wszystkie metody nie są z tego samego obszaru, wydziel je do innych klas.
1

jak masz coś w ifie- case-e whilu zaznaczasz sobie teskt, naciskach CTRL-ALT-M (mac version cmd-alt-m) i wydzielasz metodę. Później bierzesz to co Ci zostało i powtarzasz operację aż będzie ładnie. Jak masz Intellij to Ci od razu sam zniszczy duplikaty ;-)

0

krzysiek050 i wojciechmaciejewski dziękuję za sugestie. krzysiek050 jak to zrozumieć cyt. "wszystkie metody nie są z tego samego obszaru"?

PS. wojciechmaciejewski piękna jest ta opcja Ctrl+Alt+M

2

Tutaj robisz coś na wątkach. Samo zarządzanie wy wykonywanie czegoś współbieżnie jest czymś osobnym od budowania tego co przetwarzasz i faktycznego przetwarzania. Jak podzielisz kod na metody to zobaczysz które metody nie pasują do obrazka ;).

0

Po uporządkowaniu kodu zgodnie z zaleceniami wojciechmaciejewski, widać metody które rzeczywiście nie pasują do klasy. Dzięki raz jeszcze za pomoc. Kod teraz wygląda jak trzeba.

0
  1. Napisać testy jednostkowe.
  2. Wywalić całą implementację do kosza
  3. Zakomentować wszystkie testy
  4. Przepisać wszystko na nowo, odkomentowując kolejne testy

Tak chyba najprościej będzie. Choć niekoniecznie najlepiej, bo testy jednostkowe, które napiszesz, z konieczności będą odzwierciedlać wady kodu. Najlepszym sposobem byłoby raczej napisanie wszystkiego na nowo, nie trzymając się starej implementacji i starych założeń (jeśli to co wrzuciłeś to jest to cały kod, to bez problemu można tak zrobić - przecież to kawałek kodu zaledwie. Gorzej jeśli to jest np. jeden moduł z setek podobnych... Wtedy nie ruszysz tego, bo cała aplikacja złożona z dziesiątek tysięcy linijek kodu się rozwali).

Obecnie zastanawiam się jak by uporządkować ten kod, (...) czy dodać więcej metod

Jak masz burdel, to chcesz dokładać jeszcze więcej burdelu? Problem z kodem, który wrzuciłeś:

  • masz za dużo zmiennych (jak dodasz "więcej metod", to będzie jeszcze gorzej).
  • za duża złożoność cyklomatyczna (najprostszy sposób na pozbycie się: napisać to od nowa, tylko lepiej. To o wiele prostsze i mniej bugo-genne niż wydzielanie kolejnych ifów). A samo wydzielenie metod niewiele ci pomoże, bo ta złożoność dalej tam będzie.
  • funkcja nie wiadomo co robi w sumie (tutaj można by oczywiście podzielić ją na parę mniejszych, z drugiej strony jeśli teraz ją podzielisz, to nie zrobisz tego i tak w inny sposób, niż w ten, w który już jest funkcja podzielona za pomocą składni języka (chodzi o to, że jak masz np. pętlę while to zapewne mógłbyś wydzielić tę pętle bezboleśnie do jakiejś oddzielnej metody, żeby było czytelniej - tylko, że możliwe, że wcale ta pętla nie musi być potrzebna). (czyli w skrócie: moim zdaniem ten kod lepiej przepisać na nowo, lepiej, niż rzeźbić w gównie)

czy podzielić je na klasy abstrakcyjne, czy interfejsy

W zasadzie jeśli masz tam ileś zmiennych o prefiksie proxy to przypuszczalnie abstrakcja pod tytułem proxy już istnieje w kodzie, więc możliwe, że można by wydzielić jakąś klasę proxy. Ale to będzie zabieg kosmetyczny (chociaż być może potrzebny).

0

zacznij od wydzielenia odpowiedzialnosci do osobnych klas oraz metod, idac w te strone, łatwiej bedzie Ci zdecydowac ktore metody wydzielic do komponentow, na razie to tam burdel ze hoho

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