Wątek przeniesiony 2018-09-09 20:21 z Mobilne przez furious programming.

Ocena kodu - Android

0

Pisze od jakiegoś czasu na androida, za miesiąc zaczynam zaocznie studiować informatykę.
Proszę o ocenę mojego kodu w sensie wskazanie co jest do poprawy i czy mając coś takiego na koncie mogę myśleć o jakimś przynajmniej stażu?
To moja ostatnia aplikacja: https://github.com/Harkor15/AddUs
Zdaje sobie sprawę że jej kod nie jest jakiś świetny np: nie bardzo wiedziałem jak poradzić sobie z sprawami związanymi z Google play games w MVVM.

0

stwórz fake brancha, zrób do niego "pull request" master i w ten sposób wystaw review.
Będzie łatwiej komentować, więc dostaniesz dokładniejsze komentarze.

git checkout --orphan reviewHelper
git rm -rf .
echo "Dummy file for dummy commit" >dummy.txt
git add dummy.txt
git commit -m "Dummy commit for full code review"
git push origin reviewHelper

I teraz zrób pull request dla master do reviewHelper i podeślij linka.

W xml-ach polecam używać propety style. Bardzo to ładnie czyści xml-e i czyni bardzij czytelnymi.
Jak trzeba coś poprawić, np marginesy, kolory, czcionki; to poprawiasz w stylu i wszystko ładnie się zmienia.
@+id/<cos> powoduje tworzenie symbol więc powinno pojawiać się raz, w innych miejscach powinno być bez +.

1

Standard wszystkich projektów - niepotrzebny folder .idea/, niepotrzebne zależności do bibliotek testowych, niepotrzebne standardowe przykładowe testy, niepotrzebna konfiguracja ProGuarda.

Teraz co do samego projektu.

  1. Mało mówiące wiadomości w commitach i duże commity.

  2. Kiepski podział pakietów. Pakiety powinny mówić coś konkretnego o aplikacji raczej. Worki w styly "interfaces" albo "handlers" nie niosą żadnej informacji same w sobie. Klasy też wydają się być pomieszane - np. SoundsPlayer w pakiecie view.

  3. Zakomentowane linie kodu powinny być usunięte. Podobnie puste klasy, które nic nie robią (np. MainViewModel).

  4. Nie znam super dobrze DataBinding, ale chyba korzystanie jednocześnie z ButterKnife i DataBinding jest trochę dziwne. DataBinding chyba oferuje wszystko i więcej, co można wyciągnąć z ButterKnife'a.

  5. W Javie nie stosuje się raczej konwencji nazewnictwa interfejsów w stylu I-cośtam. To nie C#. Praktycznie nie ma to znaczenia, ale lepiej trzymać się konwencji. Formatowanie kodu też trochę dziwne. Brak odstępów, wszystko bardzo zwięzłe. W tym wpadku już nie chodzi o konwencję, ale o czytelnośc kodu, która spada. Może to moje przyzwyczajenie, ale wydaje mi się, że większość programistów by się zgodziła, że czytelność dramatycznie spada, gdy kod jest zbity.

  6. Kiepsko nazwane klasy (przykładowo Logic, SharedP), które nic nie mówią. Ogólnie klasa Logic mówi mi, że ktoś nie miał pomysłu, więc zróbmy jeden klocek dla logiki aplikacji. Przypomina mi się jeden projekt w którym byłem i klasa BusinessLogic, która była odpowiedzialna za wszystko.

  7. Squares.setImage() wygląda bardzo kiepsko. Po pierwsze ciężko się czyta przez formatowanie kodu, po drugie na pewno można to zrefaktorować tak, żeby nie było drabinki kodu.

     public void setImage(){
        switch(value){
            case 1: if(color==1){
                if(clicked){
                    drawable= R.drawable.square11c;
                }else{
                    drawable= R.drawable.square11;
                }
            }else{
                if(clicked){
                    drawable= R.drawable.square12c;
                }else{
                    drawable= R.drawable.square12;
                }
            }break;

            case 2: if(color==1){
                if(clicked){
                    drawable= R.drawable.square21c;
                }else{
                    drawable= R.drawable.square21;
                }
            }else{
                if(clicked){
                    drawable= R.drawable.square22c;
                }else{
                    drawable= R.drawable.square22;
                }
            }break;
            case 4: if(color==1){
                if(clicked){
                    drawable= R.drawable.square41c;
                }else{
                    drawable= R.drawable.square41;
                }
            }else{
                if(clicked){
                    drawable= R.drawable.square42c;
                }else{
                    drawable= R.drawable.square42;
                }
            }break;
            case 8: if(color==1){
                if(clicked){
                    drawable= R.drawable.square81c;
                }else{
                    drawable= R.drawable.square81;
                }
            }else{
                if(clicked){
                    drawable= R.drawable.square82c;
                }else{
                    drawable= R.drawable.square82;
                }
            }break;
            case 16: if(color==1){
                if(clicked){
                    drawable= R.drawable.square161c;
                }else{
                    drawable= R.drawable.square161;
                }
            }else{
                if(clicked){
                    drawable= R.drawable.square162c;
                }else{
                    drawable= R.drawable.square162;
                }
            }break;
            case 32: if(color==1){
                if(clicked){
                    drawable= R.drawable.square321c;
                }else{
                    drawable= R.drawable.square321;
                }
            }else{
                if(clicked){
                    drawable= R.drawable.square322c;
                }else{
                    drawable= R.drawable.square322;
                }
            }break;
        }
        Log.i("getImage","check");
    }
  1. Publiczne mutowalne pola w klasach. Publiczne mutowalne statyczne pola w klasach. Bardzo łatwo nadepnąć na minę, pracując z takimi klasami.

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