Code review aplikacji na Androida

Dzień dobry. Od kilku miesięcy uczę się pisać aplikacje na androida w języku Java. Chciałbym nabrać dobrych praktyk, ale niestety samemu jest to dla mnie trudne do osiągnięcia. Czy znalazłby się może ktoś kto zechciałby poświęcić kilka minut by spojrzeć na aplikacje które zrobiłem i ewentualnie dać kilka porad lub wskazówek?

Mój gitHub: https://github.com/divid3d/

Recenzja https://github.com/divid3d/MathRush

1. Piszesz od kilku miesięcy, a cały kod to praktycznie jeden Initial commit ;). Spróbuj udawać że faktycznie korzystasz z gita poprawnie. A najlepiej to faktycznie rób (pracując samodzielnie nie jest to jakoś kluczowe, ale pomaga - zwiększa dyscyplinę, nie ma strachu przy refactoringu, etc.)

2. Prywatnie bym wyłączył cały folder .idea z VC, ale "internet" ma różne opinie na ten temat - więc tutaj OK.
   Ale jakieś checksumy (https://github.com/divid3d/MathRush/blob/master/.idea/caches/build_file_checksums.ser) wyglądają podejrzanie.

3. Pliki .gif i inne pomocnicze przeniósłbym do jakiegoś folderu img/. Szczególnie że są używane wyłącznie do README.md.

4. https://github.com/divid3d/MathRush/blob/master/app/src/test/java/com/example/divided/mathrush/ExampleUnitTest.java dobry test, ale po co assertEquals(4, 2 + 2); w projekcie? :P
   To samo do https://github.com/divid3d/MathRush/blob/master/app/src/androidTest/java/com/example/divided/mathrush/ExampleInstrumentedTest.java

5. Pierwszy plik na który patrzyłem: https://github.com/divid3d/MathRush/blob/master/app/src/main/java/com/example/divided/mathrush/GameOverActivity.java - formatowanie dość niechlujne. Często się zdarzają dwie puste linie pod rząd, albo pusta linia bez żadnego powodu (jak ta po try)

6. Ten sam plik, komentarz //no kurwa jak to zrobic ???? ;c

7. Dużo szczegółów które dałoby się poprawić. Np w compare:

        @Override
        public int compare(ScoreInformation o1, ScoreInformation o2) {

            if (Integer.parseInt(o1.getScore()) < Integer.parseInt(o2.getScore()))
                return 1;
            else if (Integer.parseInt(o1.getScore()) > Integer.parseInt(o2.getScore()))
                return -1;
            else {
                return Integer.compare(Integer.parseInt(o2.getRound()), Integer.parseInt(o1.getRound()));
            }
        }

Niepotrzebnie parsujesz o1 i o2 bez przerwy. To wszystko mogłoby to być równie dobrze (plus minus):

Integer i1 = Integer.parseInt(o1.getScore());
Integer i2 = Integer.parseInt(o2.getScore());
if (i1 == i2) {
    return Integer.compare(...);
}
return Integer.compare(i1, i2);

8. ...wait a second, czemu w ogóle "score" jest stringiem skoro w założeniu jest zawsze Integerem? Przez to masz tylko ciągłe parsowaine i formatowanie Integer.toString(score) a potem Integer.parseInt.

9. Metoda void onCreate w tym pliku ma z 200 linijek. Większość z tego da się spokojnie przenieść do innych metod.

To już prawie 10 uwag, a dopiero z jednego pliku z kodem i nie doszedłem nawet do połowy.
Jak pewnie zauważyłeś, prawie wszystkie uwagi odnosiły się do "czystości" kodu zamiast do samego designu (a tez miałbym uwagi - np. cała logika w Activity bez żadnej abstrakcji ani wydzielania metod). To nie znaczy że się czepiam żeby się czepiać. Czysty/ładnie wyglądający kod jest wazny z wielu powodów, na przykład:

• praktyczny - w momencie kiedy jesteś jedynym programistą pracującym nad projektem nie ma wielkiego znaczenia. Ale kiedy jest ich więcej, jasno ustalone zasady których każdy się trzyma dramatycznie upraszczają pracę
• pragmatyczny - kiedy potencjalny pracodawca wejdzie na Twojego githuba, nie będize miał czasu czytać za dużo. Być może pomyśli czy design ma sens. Ale na pewno zauważy czy kod wygląda schludnie i na porządnie napisany ;).

Bardzo dziękuję za poświęcony czas oraz wszelkie uwagi. Akurat ten nowszy projekt jest bardzo niechlujny i rozwijam go na bieżąco. Czy mógłbyś jeszcze w wolnej chwili rzucić okiem na starszy projekt gdyż jest on w moim mniemaniu bardziej uporządkowany?

8. ...wait a second, czemu w ogóle "score" jest stringiem skoro w założeniu jest zawsze Integerem? Przez to masz tylko ciągłe parsowaine i formatowanie Integer.toString(score) a potem Integer.parseInt. -

Odnośnie tego punktu, z racji braków wiedzy wyszedłem z założenia, że za pomocą metody Intent.putExtra() można przekazywać jedynie String.

divid3d ! napisał(a):

Bardzo dziękuję za poświęcony czas oraz wszelkie uwagi. Akurat ten nowszy projekt jest bardzo niechlujny i rozwijam go na bieżąco. Czy mógłbyś jeszcze w wolnej chwili rzucić okiem na starszy projekt gdyż jest on w moim mniemaniu bardziej uporządkowany?

8. ...wait a second, czemu w ogóle "score" jest stringiem skoro w założeniu jest zawsze Integerem? Przez to masz tylko ciągłe parsowaine i formatowanie Integer.toString(score) a potem Integer.parseInt. -

Odnośnie tego punktu, z racji braków wiedzy wyszedłem z założenia, że za pomocą metody Intent.putExtra() można przekazywać jedynie String.

No to przecież możesz tylko w tym miejscu zmienić integer na string, a wszędzie indziej używać intów :)

Intent.putExtrat(Integer.toString(i));

TomRiddle napisał(a):

divid3d ! napisał(a):

Bardzo dziękuję za poświęcony czas oraz wszelkie uwagi. Akurat ten nowszy projekt jest bardzo niechlujny i rozwijam go na bieżąco. Czy mógłbyś jeszcze w wolnej chwili rzucić okiem na starszy projekt gdyż jest on w moim mniemaniu bardziej uporządkowany?

8. ...wait a second, czemu w ogóle "score" jest stringiem skoro w założeniu jest zawsze Integerem? Przez to masz tylko ciągłe parsowaine i formatowanie Integer.toString(score) a potem Integer.parseInt. -

Odnośnie tego punktu, z racji braków wiedzy wyszedłem z założenia, że za pomocą metody Intent.putExtra() można przekazywać jedynie String.

No to przecież możesz tylko w tym miejscu zmienić integer na string, a wszędzie indziej używać intów :)

Intent.putExtrat(Integer.toString(i));

Dziękuję za odpowiedź. Przez ten czas już zdążyłem się z tym uporać ;)