Sprawdzenie kodu, czy można go jeszcze uprościć?

0

Witam.
Stworzyłem metodę, która ma za zadanie tworzenie tablicy dwuwymiarowej. Tablica ma być wypełniana danymi pobieranymi z pliku CSV dodatkowo gdy pliku nie będzie tablica ma być wypełniana domyślnymi wartościami. Stworzyłem działający kod i chciałbym prosić o oponie i rady jak go uprościć bo jakoś mi się do końca on nie podoba. Dodam tylko że javą zajmuje się tylko hobbistycznie.
Pozdrawiam TJ

/**
     * Metoda wczytuje dane z pliku do tablicy, jeśli nie ma pliku tworzona jest domyślna
     * zawarość tablicy
     *
     * @param nowaTablica domyślna tablicaa
     */
    public void odczytDane(Object[][] nowaTablica) {
        if (NazwaPliku.equals("")) {
            JOptionPane.showMessageDialog(null, "Brak dostępu do danych!\nProgram nie zna nazwy pliku z danymi.",
                    "Brak dostępu do danych!", 1);
        } else {
            String liniaDanych;
            File testPliku = new File(NazwaPliku);
            if (testPliku.isFile()) {//sprawdzenie czy plik istnieje
                try {
                    BufferedReader odczytDane = new BufferedReader(new FileReader(NazwaPliku));
                    int nrWiersza = 0;
                    try {
                        while ((liniaDanych = odczytDane.readLine()) != null) {
                            Object[] wierszDanych = liniaDanych.split(";");
                            Object[][] nowyWierszDanych = new Object[][]{wierszDanych};
                            if (nrWiersza == 0) {
                                nowaTablica = Arrays.copyOf(nowyWierszDanych, nowyWierszDanych.length);
                            } else {
                                nowaTablica = Arrays.copyOf(nowaTablica, nowaTablica.length + 1);//powiększenie tablicy o 1
                            }
                            System.arraycopy(nowyWierszDanych, 0, nowaTablica, nowaTablica.length - 1, 1);//kopiowanie tablicy do ostatniego wiersza
                            nrWiersza++;
                        }
                    } catch (IOException ex1) {
                        JOptionPane.showMessageDialog(null, "Coś poszło nie tak, błąd ex1:\n" + ex1, "Uwaga", 1);
                    }
                } catch (FileNotFoundException ex) {
                    JOptionPane.showMessageDialog(null, "Coś poszło nie tak, błąd ex:\n" + ex, "Uwaga", 1);
                }
            }
            TablicaDanych = Arrays.copyOf(nowaTablica, nowaTablica.length);
        }
    }
1

https://en.wikipedia.org/wiki/Cyclomatic_complexity
Najwięcej by dało, jeśli byś porobił sensownie nazwane metody odpowiedzialne za określone zadania. Tak samo mógłbyś połączyć tamte bloki try-catch w jeden blok łapiący IOException - FileNotFoundException to IOException. Sądzę, że Twój zamysł na początku pisania tej metody był inny - kompletnie bez przyczyny przekazujesz do niej tablicę, a potem ustawiasz globalną wartość na wynik odczytu. I masz racje, to jest bardzo dziwne i nieczytelne.

nowaTablica = Arrays.copyOf(nowaTablica, nowaTablica.length + 1);//powiększenie tablicy o 1 

Nie zmieniasz tym rozmiaru tablicy, ale tworzysz całkowicie nową tablicę o większym rozmiarze. Z tego powodu tablica przekazana do metody nie jest zmieniona - już po pierwszej iteracji tracisz do niej referencje i zapisujesz dane do nowych. Możesz albo zwracać nową tablicę, albo skorzystać z listy.

1

Można, a nawet trzeba.
6 poziomów zagłębienia nie wróży niczego dobrego.

Podziel to na mniejsze metody.

1
/**
     * Metoda wczytuje dane z pliku do tablicy, jeśli nie ma pliku tworzona jest domyślna
     * zawarość tablicy
     *
     * @param nowaTablica domyślna tablicaa
     */

To jest niepotrzebne.

        if (NazwaPliku.equals("")) {
            JOptionPane.showMessageDialog(null, "Brak dostępu do danych!\nProgram nie zna nazwy pliku z danymi.",
                    "Brak dostępu do danych!", 1);
        }

Lepiej rzuć wyjątek i obsłuż go gdzie indziej.

else {
            String liniaDanych;

Zmienna powinna być deklarowana zaraz obok pierwszego użycia.

if (testPliku.isFile()) {//sprawdzenie czy plik istnieje

Bezsensowny komentarz.

                try {
                    BufferedReader odczytDane = new BufferedReader(new FileReader(NazwaPliku));
                    int nrWiersza = 0;
                    try {
                        while ((liniaDanych = odczytDane.readLine()) != null) {
                            Object[] wierszDanych = liniaDanych.split(";");
                            Object[][] nowyWierszDanych = new Object[][]{wierszDanych};
                            if (nrWiersza == 0) {
                                nowaTablica = Arrays.copyOf(nowyWierszDanych, nowyWierszDanych.length);
                            } else {
                                nowaTablica = Arrays.copyOf(nowaTablica, nowaTablica.length + 1);//powiększenie tablicy o 1
                            }
                            System.arraycopy(nowyWierszDanych, 0, nowaTablica, nowaTablica.length - 1, 1);//kopiowanie tablicy do ostatniego wiersza
                            nrWiersza++;
                        }

Komentarze do wywalenia. Ten cały kod powinien być w innym miejscu, i zostać jeszcze podzielony na dwie lub trzy mniejsze metody. I być może sensowniej byłoby użyć javowego try with resources.

                    } catch (IOException ex1) {
                        JOptionPane.showMessageDialog(null, "Coś poszło nie tak, błąd ex1:\n" + ex1, "Uwaga", 1);
                    }
                } catch (FileNotFoundException ex) {
                    JOptionPane.showMessageDialog(null, "Coś poszło nie tak, błąd ex:\n" + ex, "Uwaga", 1);
                }

Te wyjątki powinny być obsłużone w innym miejscu.

TablicaDanych = Arrays.copyOf(nowaTablica, nowaTablica.length);

A to co?

0

Witam ponownie.
Wiem że nie do wszystkich zaleceń się zastosowałem, a czy teraz już lepiej to napisane niż wcześniej?

public class TabelaMetody2D {

    private Object[][] TablicaDanych = null;
	private String NazwaPliku = "";
	
	public TabelaMetody2D(String NazwaPlikuDanych) {
        NazwaPliku = NazwaPlikuDanych;
    }

    public Object[][] odczytDaneTablica() {
        return TablicaDanych;
    }
	
	public void odczytDane2(Object[][] domyslnaTablica) {
        int nrWiersza = 0;
        String liniaDanych;
        BufferedReader ileDanych = null;
        if (NazwaPliku.equals("")) {
            JOptionPane.showMessageDialog(null, "Brak dostępu do danych!\nProgram wczytuje domyślną zawartość tablicy.",
                    "Brak dostępu do danych!", 1);
        } else if (czyPlikIstnieje() == true) {
            Object[][] nowaTablica = new Object[ileWierszyTablica()][domyslnaTablica[0].length];
            try {
                ileDanych = new BufferedReader(new FileReader(NazwaPliku));
                while ((liniaDanych = ileDanych.readLine()) != null) {
                    Object[][] nowyWierszDanych = new Object[][]{liniaDanych.split(";")};
                    System.arraycopy(nowyWierszDanych, 0, nowaTablica, nrWiersza, 1);
                    nrWiersza++;
                }
            } catch (FileNotFoundException ex) {
                System.err.println("Błąd nr1: " + ex);
            } catch (IOException ex) {
                System.err.println("Błąd nr2: " + ex);
            }
            TablicaDanych = Arrays.copyOf(nowaTablica, nowaTablica.length);
        } else {
            TablicaDanych = Arrays.copyOf(domyslnaTablica, domyslnaTablica.length);
        }
    }

    private boolean czyPlikIstnieje() {
        File testPliku = new File(NazwaPliku);
        return testPliku.isFile();
    }

    private int ileWierszyTablica() {
        int ileWierszy = 0;
        try {
            String liniaDanych;
            BufferedReader ileDanych = null;
            ileDanych = new BufferedReader(new FileReader(NazwaPliku));
            while ((liniaDanych = ileDanych.readLine()) != null) {
                ileWierszy++;
            }
        } catch (FileNotFoundException ex) {
            System.err.println("Błąd nr1: " + ex);
        } catch (IOException ex) {
            System.err.println("Błąd nr2: " + ex);
        }
        return ileWierszy;
    }    
}

Pozdrawiam TJ

0
private Object[][] TablicaDanych = null;

Domyślnie będzie null.

private String NazwaPliku = "";

na

private final String NazwaPliku;

Bo i tak inicjalizujesz w konstruktorze.

int nrWiersza = 0;
        String liniaDanych;
        BufferedReader ileDanych = null;

co Ty masz z tym deklarowaniem zmiennych na zapas? Źle.

if (NazwaPliku.equals("")) {
            JOptionPane.showMessageDialog(null, "Brak dostępu do danych!\nProgram wczytuje domyślną zawartość tablicy.",
                    "Brak dostępu do danych!", 1);
        }

Zamiast tego rzuć wyjątek, i obsłuż go gdzie indziej.

} catch (FileNotFoundException ex) {
                System.err.println("Błąd nr1: " + ex);
            } catch (IOException ex) {
                System.err.println("Błąd nr2: " + ex);
            }

Same.

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