Prośba o code review

Odpowiedz Nowy wątek
2015-09-26 22:21

Rejestracja: 4 lata temu

Ostatnio: 4 lata temu

0

Słowem wstępu. Zajmuje się programowaniem od początku tego roku. Napisalem prosty programik wg. otrzymanych specyfikacji, ktory konsumuje API archiwum Narodowego Banku Polskiego. Program pobiera kursy sprzedaży i kursy kupna dla danej waluty w określonym przedziale czasowym, a potem liczy sobie z tego średnią kursów kupna i odchylenie standardowe kursów sprzedaży. Tutaj moja implementacja.

Nie zajmowałem się jeszcze wątkami, dlatego aplikacja działa wolno (musiałem pobierać daty z plików xml). Prosty workaround polegałby na użyciu innego Predicate w metodzie getXmlFileNames(Predicate<string> predicate), który selekcjonowałby pliki jeszcze przed ich parsowaniem. Cel na najbliższe dni, to lektura "Concurrency in practice" i poprawa wydajności. No ale zanim to nastąpi, byłbym wdzięczny jeśli ktoś znalazłby chwilę na skomentowanie tego co widzi.

Od siebie dodam, że mam problem z testami. Im dalej, tym kod robi się coraz bardziej nieczytelny. Problem widać w DailyRatingFilterTest. Jest jakiś sens pisania testów dla XmlParserImp? W końcu ta klasa jest tylko wraperem dla zewnętrznych bibliotek. Przydałyby sie też testy dla ContextBuilder'a, ale w obecnej sytuacji byłby tam niezły misz-masz.

edytowany 1x, ostatnio: sdwsk, 2015-09-26 22:23
jak od początku tego roku i od zera to dosyć spoko ;] - karolinaa 2015-09-26 22:42
no tak jak piszesz ;] - sdwsk 2015-09-26 23:29

Pozostało 580 znaków

2015-09-26 22:38

Rejestracja: 7 lat temu

Ostatnio: 4 lata temu

Lokalizacja: Bieszczady

4

1.

    public List<Record> getContent() {
        if (content == null) {
            content = new ArrayList<Record>();
        }
        return this.content;
    } 

2.
zapominasz(albo robisz to celowo) access modifiers https://github.com/sdwsk/nbp/[...]p/services/DirCrawlerImp.java

3.
Te Twoje metody robią trochę za dużo miejscami, np tutaj:
https://github.com/sdwsk/nbp/[...]nbp/services/DirSelector.java

wolałbym widzieć np:

    public List<String> getXmlDirs() {
        List<String> nbpDirectoryNames = new ArrayList<>();
        costammetodacosasd(nbpDirectoryNames );
        jakasmetodacosasdad(nbpDirectoryNames );

        return nbpDirectoryNames;
    }

4.
weź tam wepnij jakieś DI, Springa albo tego Guice czy Picko

Piszesz coś o wielowątkowości, zacznij od razu od ludzkiej strony
https://docs.oracle.com/javas[...]urrent/CompletableFuture.html

P.S
@karolinaa chciała też zaproponować że jak chcesz to napiszę Ci ten program na nowo, za drobną opłatą :D


"Perhaps surprisingly, concurrent programming isn’t so much about threads or
locks, any more than civil engineering is about rivets and I-beams."
edytowany 3x, ostatnio: niezdecydowany, 2015-09-26 22:44
tak mogę napisać :D - karolinaa 2015-09-26 23:00

Pozostało 580 znaków

2015-09-26 23:20

Rejestracja: 4 lata temu

Ostatnio: 4 lata temu

0

1) To akurat kod automatycznie wygenerowany przez JAXB na podstawie .xsd, nie zagladalem tam poza jedna mala modyfikacja.
2) Zapominam, widze ze musze sie bardziej nad tym skoncentrowac.
3) Racja
4) Mialem zamiar wpiac Springa po watkach.
P.S ;)

edytowany 3x, ostatnio: sdwsk, 2015-09-27 00:15

Pozostało 580 znaków

Odpowiedz

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