Code review - Endoscope wi fi live streamer (ANDROID)

Napisałem swoją drugą aplikacje na urządzenia z androidem. Dzięki Endoscope możesz połączyć bardzo szybko dwa urządzenia android następnie przesyłać pomiędzy nimi obraz na żywo z kamery za pomocą sieci wi-fi.

Jedno urządzenie pełni rolę hosta (server RTSP) streamującego a drugie łapie stream. Wszystko za pomocą protokołu RTSP. Połączenie dwóch urządzeń może odbyć się za pomocą:

• QR CODE - jednym urządzeniem zeskanuj kod qr code na w którym zapisany jest adres ip.
• NFC - aktywuj nfc następnie przyłóż urządzenia do siebie aby przesłać adres ip.
  lub wpisać adres ręcznie.

Szukam code review, cały projekt jest open source na githubie: https://github.com/hypeapps/Endoscope

Kodu nie sprawdzę, ale pomysł mi się podoba; No i na zrzutach widać, że interfejs ładny :]

Masz zamiar rozwijać dalej ten program, czy naklepałeś go jedynie do portfolio?

To tylko projekt do nauki oraz portfolio, nie zamierzam go już rozwijać. Będę teraz realizował kolejny projekt przy którym chce nauczyć się RESTa.

No dobra, ja się mogę podjąć.

1. https://corner.squareup.com/2014/10/advocating-against-android-fragments.html Jeśli masz jakieś konkretne powody dlaczego używasz fragmentów, odbij piłkę.
2. Brak architektury. Posiadasz activity, fragmenty, adaptery. Brak Presenterów, modeli. Fajnie można się pobawić MVP.
3. Brak testów

if(ipAddressTextView!= null) ipAddressTextView.setText(ipAddress);

Brak klamerek, brak else. W else można by zrobić ErrorHandling
5. InfoNfcFragment createRecord. Trochę nieczytelna ta metoda.
6. Widzę sporo miejsc gdzie masz za dużo enterów
7. Pomyśl o użyciu ButterKnife, żeby uniknąć findViewById. Swoją drogą fajny wstęp do Dependency Injection (Dagger2)
8.

 ArrayList<String> messagesReceivedArray

. List<String> messagesReceivedArray

 tak to powinno być
9. 
```java
 public void SlideToNfcPage(View view){
        viewPager.setCurrentItem(2);
    }

camelCase, magiczna liczba. Nie wiem co to jest 2.
10. handleNfcIntent jak w pkt 5
11. onNewIntent to już chyba stara metoda.
12. HowToUseActivity brak modyfikatora dla ViewPager, tak samo dla kilku metod
13. HowToUsePagerAdapter, getCount 4. A co jeśli będzie więcej elementów?
14. PagerCirclesManager dotStatusManage tu jest DRY
15. SettingsActivity brak modyfikatorów dla pól. Do dialogów możesz zrobić jakiś generator.
16. StartStreamPagerAdapter tak jak w pkt 13
17. ViewStreamPagerAdapter j/w

Nie ma error handlingu, nie ma mvp, nie ma testów. Takie rzeczy jak blokowanie orientacji powinno być w Manifeście. Przynajmniej przerabiając apkę, tam bym się tego spodziewał. Nie pochyliłem się jeszcze nad SplashScreenem bo nie mam czasu, ale same handlery do zmiany aktywności są słabym pomysłem.

Tyle.

@panryz Świetnie. Tego właśnie szukałem! Dziękuje. Przeanalizuje kod dokładnie pod kątem tych uwag. Co proponujesz zamiast handlerów?

Byłby ktoś odważny przejrzeć frontend?

Hajp napisał(a):

Szukam code review, cały projekt jest open source na githubie: https://github.com/hypeapps/Endoscope

1. Przede wszystkim struktura aplikacji i jej architektura to część nad którą można popracować. Poczytaj o wzorcu MVP. Wydziel odpowiednie warstwy. Pro tip - postaraj się tak zrobić aby w Presenterze było jak najmniej importów z Androida.
2. Jak masz metody po nawigacji po aplikacji to stwórz klasę 'Navigator' z prywatnym konstruktorem oraz metodami statycznymi. W ten sposób stworzysz prostą klasą do nawigowania po całej aplikacji i możesz w niej wrzucać metody które będą przenosić do danych aktywności.
3. Zastosuj bibliotekę 'ButterKnife' ułatwia inicjalizację kontrolek oraz eliminuje anonimowe z kodu, (klasy anonimowe są zastąpione.
4. Nie pisz zmiennych jednoliterowych zmiennych typu 'v'.
5. Tworzysz obiekt SharedPreferences w wielu miejscach. Pomyśl nad odpowiednią klasą której zadaniem by bylo tylko i wyłącznie zapisywanie lub odczyt w SharedPreferencas. Staraj tworzyć klasy odnośnie funkcjonalności. Jak aktwywność robi różne rzeczy typu zapisuje i odczytuje dane, obsługuje widok itp to jest złe. Łamiesz zasadę pojedynczej odpowiedzialności przez co taka klasa jest trudna do testowania.
6. Usuwaj puste linie.

To mi się wrzuciło w oczy na początku. Pomysł aplikacji i jak jej wygląd na plus. Pobaw się trochę kodem (jeśli powtarzasz się w wielu miejscach z tą samą rzeczą zastanów się czy nie warto pobawić się tutaj abstrakcją albo zadanie oddelegować do odpowiedniej klasy).

1. Stringów nie piszemy z caplocka, masz do tego odpowiednie atrybuty w kontrolkach.
2. Puste linie w layoutach xml.
3. Staraj się unikać rozbudowanych struktur layoutów, odbija się to na wydajności (sprawdź rysowanie za pomocą odpowiednich narzędzi do analizy).
4. Pomyśl nad usystematyzowaniem nazewnictwa id dla elementów w plikach xml. Np jak masz TextView sigin to nazwij go tv_sign_in (notacja węgierska).

Jeszcze jedno. Jak masz w layoutach jakiekolwiek ustawione szerokości i wysokości za pomocą wartości liczbowych to jest wyeksportuj do pliku dimens.