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.
Ocena kodu - Android
Wątek przeniesiony 2018-09-09 20:21 z Mobilne przez flowCRANE.
- Rejestracja: dni
- Ostatnio: dni
- Postów: 13
- Rejestracja: dni
- Ostatnio: dni
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 +.
- Rejestracja: dni
- Ostatnio: dni
- Lokalizacja: Kraków
- Postów: 834
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.
-
Mało mówiące wiadomości w commitach i duże commity.
-
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.
SoundsPlayerw pakiecieview. -
Zakomentowane linie kodu powinny być usunięte. Podobnie puste klasy, które nic nie robią (np.
MainViewModel). -
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.
-
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. -
Kiepsko nazwane klasy (przykładowo
Logic,SharedP), które nic nie mówią. Ogólnie klasaLogicmó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 klasaBusinessLogic, która była odpowiedzialna za wszystko. -
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");
}
- Publiczne mutowalne pola w klasach. Publiczne mutowalne statyczne pola w klasach. Bardzo łatwo nadepnąć na minę, pracując z takimi klasami.