JAVA - Refaktoryzacja Sanityzatora kodu

Question

  public static void main(String args[]){
        Instaling L=new Instaling();
        L.spliting_word();

    }

Napisałem prosty program dzielący tekst na części i wkładajcy je do odpowiednio zmiennych (np:. 1. all - wszystko na word_ang = all , word_pl = wszytsko), jestem jednak początkującym programista Javy i nie ogarniam co można byłoby w nim poprawic, aby składniowo był lepszy. Czy da się go jakoś poprawić , I czy ktos mogłby pokazac jak ewentualnie na co zwrócić uwage ?

        private String [] word=new String[]{
                "0. word_ang - word_pl\n" + "1. gap year / take a gap year   -   rok przerwy po ukonczeniu szkoly sredniej\n" +
                        "2. safety course   -   kurs zasad bezpieczenstwa\n" + "3. increase   -   zwiekszac sie (o ilosci/jakosci)\n" +
                        "4. decrease   -   zmniejszac sie (o ilosci/jakosci)\n" +
                        "5. within walking distance   -   w odleglosci, ktora mozna pokonac pieszo\n" +
                        "6. mention   -   wspomniec o czyms\n" + "7. overland   -   droga ladowa / naziemny\n" +
                        "8. memorable   -   niezapomniany/ godny zapamietania\n" +
                        "9. sign up with   -   zapisac sie/zaangazowac/zwiazac sie z \n" +
                        "10. deliverer   -   doreczyciel/dostawca\n" + "11. paragliding   -   paralotnia\n" + "12. placement   -   polozenie/lokalizacja\n" +
                        "13. research   -   badania naukowe\n" + "14. expences   -   wydatki\n" + "15. cruise ship   -   statek rejsowy\n" +
                        "16. on board   -   na pokladzie\n" + "17. off the coast   -   z daleka od brzegu\n" + "18. available   -   dostepne\n" +
                        "19. working hours   -   godziny pracy\n" + "20. deal with   -   radzic sobie z\n" + "21. counsellor   -   doradca/opiekun\n" +
                        "22. freelance   -  pracowac jako wolny strzelec\n" + "23. call back   -   oddzwonic pozniej\n" + "24. take on   -   zatrudniac\n" +
                        "25. hold a certificate   -   posiadac dyplom\n" + "26. put through   -   polaczyc przez telefon\n" + "27. flood   -   powodz\n" +
                        "28. volcanic eruption   -   wybuch wulkanu\n" + "29. drought   -   susza\n" + "30. pollution   -  zanieczyszczenie\n" +
                        "31. earthquake   -   trzesienie ziemi\n" + "32. pump out   -   wypompowywac/emitowac\n" + "33. toward   -   w kierunku\n" +
                        "34. grow   -  rosnac/uprawiac\n" + "35. canoe   -   kajak\n" + "36. fireworks   -   fajerwerki\n" +
                        "37. light up   -  rozswietlic\n" + "38. sheke   -  trzasc sie\n" + "39. fall off   -   spadac/odpadac\n" +
                        "40. twister   -   tornado/traba powietrzna\n" + "41. calm   -   spokojny/nieporuszony\n" +
                        "42. weatherman   -   prezenter pogody\n" + "43. strike   -   uderzyc/nadejsc/dopasc\n" +
                        "44. pray   -   modlic sie\n" + "45. prayer   -   modlitwa\n" + "46. put down   -   wpisywac/klasc\n" +
                        "47. huge   -   ogromny/olbrzymi\n" + "48. directly   -  bezposrednio\n" + "49. unexpected   -   nieoczekiwany\n" +
                        "50. expected   -   oczekiwany\n" + "51. chase   -   gonic/scigac\n" +
                        "52. awake   -   obudzony/przebudzony/obudzic/przebudzic sie\n" +
                        "53. prediction   -   przewidywanie\n" + "54. predict   -   przewidywac\n" +
                        "55. spontaneous decision   -   decyzja spontaniczna\n" + "56. global warming   -   globalne ocieplenie\n" +
                        "57. rising level   -   podnoszacy sie poziom\n" +
                        "58. cover   -   przykrywac/pokrywac/przykrywa/pokrywa (czasownik i rzeczownik)\n" + "59. glacier   -   lodowiec\n" +
                        "60. well   -   studnia\n" + "61. cause   -   powodowac\n" + "62. precious   -   drogocanny\n" +
                        "63. collect donations   -   zbierac datki\n" + "64. fossil fuel   -   paliwo kopalne\n" +
                        "65. ice cap   -   pokrywa lodowa\n" + "66. blanket   -   koc\n" + "67. shortage   -   brak/niedobor\n" +
                        "68. leak   -   przeciekac/wyciek\n" + "69. dispose of   -   unieszkodliwic/uporac sie z\n" +
                        "70. dump   -   sklad/zrzut\n" + "71. breed   -   rasa/gatunek\n" + "72. die out   -   wymrzec/wyginac\n" +
                        "73. scarce   -   rzadki/malo spotykany\n" + "74. look for   -   szukac\n" + "75. look after   -   opiekowac sie\n" +
                        "76. cub   -   kocie/mlode\n" + "77. cuddly   -   milutki\n" + "78. fierce-looking   -   wygladajacy groznie\n" +
                        "79. habitat   -   srodowisko/miejsce wystepowania\n" + "80. in case of   -   w razie/jesli wystapi\n"

        };
        private int random=81;

        private int loop=19;

        private String [] rand_same = new String [loop+1];
        private String user;
        private int[] rand = new int[loop+1];
        private Scanner scanner = new Scanner(System.in);
        private int[] miss = new int[loop+1];
        private int m = 0;
        private int i = 0;
        private int j = 0;

        private String[] row_spliter=new String[random];
        private String[] word_pl = new String[random];
        private String[] word_ang = new String[random];



        public void spliting_word(){

            for(int i=0;i<=random-1;i++)
            {

                String [] split_n=word[0].split("\n");
                for(int j=0;j<=random-1;j++)
                {
                    row_spliter[j]=split_n[j];

                }
            }
            //dzielenie tablic
            for(int i=0;i<=random-1;i++) {
                row_spliter[i] = row_spliter[i].replace(i+". ","");
            }
            String[][] wordsArray = new String[2][random];
            for(int i=0;i<=random-1;i++) {
                String[] word_split = row_spliter[i].split(" - ");
                for(int j=0;j<word_split.length;j++){
                    wordsArray[j][i]= word_split[j];
                }
            }
            //sanityzacja podzielonych tablic
            for(i=0;i<=random-1;i++) {
                word_ang[i]=wordsArray[0][i];
                word_pl[i]=wordsArray[1][i];
                if(word_ang[i].contains("/"))
                {
                    word_ang[i]=word_ang[i].substring(0,word_ang[i].indexOf("/"));
                }

                while(word_ang[i].endsWith(" ")||(word_pl[i].startsWith(" "))) {
                    if (word_ang[i].endsWith(" ")) {
                        word_ang[i] = word_ang[i].substring(0, word_ang[i].length() - 1);
                        //System.out.println(word_ang[i]);
                    }
                    if (word_pl[i].startsWith(" ")) {
                        word_pl[i] = word_pl[i].replaceFirst(" ", "");
                        //System.out.println(word_ang[i]);
                    }
                }
            }
        }

 

Comments

Zagłębiałeś się kiedyś w zasady czystego kodu w jakimkolwiek języku programowania?

---

Jestem nowy jesli chodzi o sanityzacje kodu to moj pierwszy wiekszy projekt

Answer 1

Na szybko zmienne o nazwach m, i, j... nic nie mówią. Nie używaj takich zmiennych. W twoim przypadku w  liniach 48-50 tworzysz zmienne... których nie używasz. Nie używasz ich dlatego, że w pętli masz inne zmienne, którym tam na nowo przypisujesz wartości początkowe. Dla mnie linie 48-50 możesz usunąć i wtedy nie musisz myśleć nad nazwami dla i, j, m.

Kolejna sprawa czy 

for(int j=0;j<=random-1;j++)

nie jest tym samym co ?

for(int j=0;j<random;j++)

Druga opcja jest czytelniejsza :)

Trzecia - według mnie rzecz do poprawy z największym priorytetem - linia 82 do konca. For, if, while, if... to nie jest czytelne i powinieneś to podzielić na funkcje z mniszymi funkcjonalnościami. 

Comments

Dzieki za porade. Linie 48-50 są używane tylko w innej czesci kodu której tutaj nie pokazałem a zmienne i m j jako iteratory w nich. Oraz chodzi ci o cos takiego ? 

public void spiltingArrayToWord() {
            for (int i = 0; i < random; i++) {
                String[] split_n = word.split("\n");
                for (int j = 0; j < random; j++) {
                    row_spliter[j] = split_n[j];
                }
            }
        }

        public void dividingWordToPlOrAng() {
            //dzielenie tablic
            for (int i = 0; i < random; i++) {
                row_spliter[i] = row_spliter[i].replace(i + ". ", "");
            }
            String[][] wordsArray = new String[2][random];
            for (int i = 0; i < random; i++) {
                String[] word_split = row_spliter[i].split(" - ");
                for (int j = 0; j < word_split.length; j++) {
                    wordsArray[j][i] = word_split[j];
                }
            }
            //sanityzacja podzielonych tablic
            for (i = 0; i < random; i++) {
                word_ang[i] = wordsArray[0][i];
                word_pl[i] = wordsArray[1][i];

//                wordAng.add(wordsArray[0][i]);
//                wordPl.add(wordsArray[1][i]);
            }
        }

        public void sanitationWord() {
            for (i = 0; i < random; i++) {
                if (word_ang[i].contains("/")) {
                    word_ang[i] = word_ang[i].substring(0, word_ang[i].indexOf("/"));
                }

                while (word_ang[i].endsWith(" ") || (word_pl[i].startsWith(" "))) {
                    if (word_ang[i].endsWith(" ")) {
                        word_ang[i] = word_ang[i].substring(0, word_ang[i].length() - 1);
                        //System.out.println(word_ang[i]);
                    }
                    if (word_pl[i].startsWith(" ")) {
                        word_pl[i] = word_pl[i].replaceFirst(" ", "");
                        //System.out.println(word_ang[i]);
                    }
                }
//                wordAng.add(word_ang[i]);
//                wordPl.add(word_pl[i]);
//                System.out.println(Arrays.toString(list.toArray(wordAng.get(i))));

            }
        }

 

---

Właśnie o to mi chodzi, możesz spokojnie usunąc linie 48-50. Java ma to do siebie, że możesz zmienne lokować dynamicznie w pętli i ty to właśnie robisz:

for (int i = 0; i < random; i++)

Tutaj i jest tworzone dynamicznie i od razu ma wartość 0, nie potrzebujesz robić coś takiego

int i =0;
for (int i = 0; i < random; i++)

 

---

Jasne dzięki. Widzisz cos jeszcze co mógłbym poprawic ?

---

Nazwy zmiennych sa dla mnie nieczytelne np loop... nie rozumiem po co ta zaminna jest, może lepiej jakbyś tą zmienną nazwał loopConut? To samo ze zmienną ranodm oraz miss. 

na szybko znalazlem taki artykuł i widze, ze jest warty przeczytania.