Ksiazka clean code a jakosc kodu

Zaczalem czytac ta ksiazke, a tam informacje by robic duzo malych metod i nazwy metod mowiace co one robia itd. (niby sie to wszystko wie, ale w praniu roznie wychodzi) i biorac sobie to bardzo do serca napisalem troche kodu, jak oceniacie jego jakosc ?
Ten fragment wyszukuje w danych wejsciowych (ogromym stringu) innych stringow i zapisuje do list tokenow informacje o tym

np. " "ala" dfsd fdsff " "

znajdzie poprawny string ala i reszte uzna za bledne

mysle ze wiadomo co robi program, a o to fragment kodu...

ide po znaku z ogromnego stringa i szukam pierwszego cudzyslowu, jak znajde do gry wkracza mestoda sprawdzStringa()

 while (i < dlugoscTekstu) {
            aktualnyZnak = tworzStringaZPojedynczegoZnakuZDanych(i);

            if (jestZnakiemCharakterystycznymDlaStringa(aktualnyZnak)) {
                sprawdzStringa();
                
            }

...


        }

    }


    private void sprawdzStringa() {
        String odnalezionyStringWDanych = "\"";
        i++;
        String aktualnyZnak = tworzStringaZPojedynczegoZnakuZDanych(i);

        while (!(jestZnakiemCharakterystycznymDlaStringa(aktualnyZnak)) && czyNieJestemPoZaKoncemDanychDoSkanowania()) {
            odnalezionyStringWDanych += aktualnyZnak;
            aktualnyZnak = tworzStringaZPojedynczegoZnakuZDanych(i);
            ++i;
        }
        odnalezionyStringWDanych += aktualnyZnak;
        dodajOdpowiedniToken(odnalezionyStringWDanych);
    }
 
private boolean jestZnakiemCharakterystycznymDlaStringa(String aktualnyZnak) {
        return aktualnyZnak.equals("\"");
    }

    private boolean czyNieJestemPoZaKoncemDanychDoSkanowania() {
        return i < daneDoSkanowania.length();
    }

    private String tworzStringaZPojedynczegoZnakuZDanych(int i) {
        return String.valueOf(daneDoSkanowania.charAt(i));
    }

    private boolean jestToPoprawnyString(String odnalezionyStringWDanych) {
        return (jestZnakiemCharakterystycznymDlaStringa(tworzStringaZPojedynczegoZnakuZDanych(i - 1)) && (odnalezionyStringWDanych.length() > 1));
    }

private void dodajOdpowiedniToken(String odnalezionyStringWDanych) {
          if (jestToPoprawnyString(odnalezionyStringWDanych)) {
            tokeny.add(new Tokenek(odnalezionyStringWDanych, "String", linia));
        } else {
            tokeny.add(new Tokenek(odnalezionyStringWDanych, "Bledny_String", linia));
        }
    }


 

czy tak ma wygladac prosty, samodokumentujacy sie kod ?

Język polski nie nadaje się do kodzenia.. ;)

bolson napisał(a):

Język polski nie nadaje się do kodzenia.. ;)

no ok :), ale jakby zamiast tego przetluamczyc nazwy metod na angielski, to jest to ok ?

Przekolorowałeś to i to strasznie, a książka na prawdę daje bardzo dobre rady. A ten kod który pokazałeś jest przegadany i niezrozumiały. Każda metoda powinna mieć kilkanaście, może z 20 linijek kodu, nie jedną albo dwie, poza pewnymi szczególnymi przypadkami.
Przeczytaj CAŁĄ książkę, a potem zacznij pisać kod stosując sie do rad.

no wiem, ze dlugosc tych nazw jest straszna...

ale w tej ksiazce wlasnie autor takie polecal i wrecz mowil, ze nazwa metody ma zastepowac komentarz opisujacy co ona robi, tak by po roku czasu po przeczytaniu anzwy metody od razu wiedziec co sie dzieje...

Nie. Zdecydowanie jest zbyt.. "verbose".

Kodu nie tworzy się zawsze i bezwzględnie według jakiejś tam zasady, którą ktoś podał, a ty ją nie do końca zrozumiałeś. Kod tworzy się na zdrowy rozsądek. W taki sposób, że inny programista ogarniając kod wzrokiem zaraz go zrozumie. Nazwy metod oczywiście pomagają, ale jeżeli ciało metody, takie jak: i < daneDoSkanowania.length(); albo aktualnyZnak.equals("\"") da się przeczytać szybciej niż jej nazwę to.. coś jest nie tak ;).

ok, a tak juz jest ok: (nie liczac polskich nazw)

while (i < dlugoscTekstu) {
            aktualnyZnak = String.valueOf(daneDoSkanowania.charAt(i));

            if (aktualnyZnak.equals("\"")) {
                sprawdzStringa();

...

}


 private void dodajOdpowiedniToken(String odnalezionyString) {
        if ((String.valueOf(daneDoSkanowania.charAt(i - 1)).equals("\"")) && (odnalezionyString.length() > 1)) {
            tokeny.add(new Tokenek(odnalezionyString, "String", linia));
        } else {
            tokeny.add(new Tokenek(odnalezionyString, "Bledny_String", linia));
        }
    }

    private void sprawdzStringa() {
        String odnalezionyString = "\"";
        i++;
        String aktualnyZnak = String.valueOf(daneDoSkanowania.charAt(i));

        while (!(aktualnyZnak.equals("\"")) && i < daneDoSkanowania.length()) {
            odnalezionyString += aktualnyZnak;
            aktualnyZnak = String.valueOf(daneDoSkanowania.charAt(i));
            ++i;
        }
        odnalezionyString += aktualnyZnak;
        dodajOdpowiedniToken(odnalezionyString);
    }

tokeny.add(new Tokenek(odnalezionyString, "Bledny_String", linia));
To "Bledny_String" nie dało by się na jakiś enum, lub zwyczajny bool zamienić? Domyślam się że jedyne opcje to "String" i "Bledny_String"
A w ogóle dało by się bez tego ifa i elsa zrobić, pomyśl jak ;)

Co do nazwy "dodajOdpowiedniToken" myślę że starczyło by dodajToken, w końcu wiadomo, że ma być odpowiedni ;) ale tylko tak mówię, w porównaniu z poprzednim kodem jest dużo lepiej
O "sprawdzStringa" mozna by powiedziec, ze skoro ma cos sprawdzac, to powinna zwracac np typ boolowski (wynik pozytywny, negatywny), ale to już czepiam się :P

Guzik a nie zrobione. Co to niby znaczy "sprawdzStringa"? Mnie naazwa tej metody absolutnie nic nie mówi. Co więcej, ta metoda nie ma ani argumentów ani nic nie zwraca, to jak mi niby ma zwrócic informacje o tym co sprawdziła? Ten while jest bez sensu. Nie dało się tak:

for (char aktualnyZnak : daneDoSkanowania.toCharArray()) {
  if(aktualnyZnak=='\"'){
    break;
  }
  odnalezionyString += aktualnyZnak;
}

? Przynajmniej widać że jest tu iteracja jakaś...

Shalom napisał(a):

Guzik a nie zrobione. Co to niby znaczy "sprawdzStringa"? Mnie naazwa tej metody absolutnie nic nie mówi. Co więcej, ta metoda nie ma ani argumentów ani nic nie zwraca, to jak mi niby ma zwrócic informacje o tym co sprawdziła? Ten while jest bez sensu. Nie dało się tak:

for (char aktualnyZnak : daneDoSkanowania.toCharArray()) {
  if(aktualnyZnak=='\"'){
    break;
  }
  odnalezionyString += aktualnyZnak;
}

? Przynajmniej widać że jest tu iteracja jakaś...

to jest chyba umowna kwestia, co jest czytelne, a co nie, bo np. to co napisales nie widze by bylo jakies znowu czytelniejsze, a poza tym nie dziala dobrze, bo tutaj daneDoSkanowania sa zawsze przegladane od poczatku, a nie o to tutaj chodzi...