Operowanie na zmiennych instancji czy zwracanie wyników?

0

Mam sobie niewielką klasę FileLoader, która ma za zadanie załadować wszystkie pliki z jara, których nazwa jest zakończona ".txt" do seta. Który z poniższych wariantów jest ogólnie preferowany? Domyślam się, że 1, bo prostszy i nie komplikuje się przy kilku zagłębionych metodach (z tym miałem problem przy drugim rozwiązaniu kiedyś). Z drugiej strony wariant nr 2 pozwala na oznaczenie pola files jako final i wydaje się bardziej funkcyjnym (?) rozwiązaniem.

(pseudokod)
1:

public class FileLoader {

    private Set<Path> files;

    public FileLoader(){
        loadTextFiles();
    }

    private void loadTextFiles(){
        this.files = załaduj_pliki_z_jara();
    }
}

2:

public class FileLoader {

    private final Set<Path> files;

    public FileLoader(){
        files = loadTextFiles();
    }

    private Set<Path> loadTextFiles(){
        return załaduj_pliki_z_jara();
    }
}
0

Imho w ogóle nie powinieneś tego wywoływać z konstruktora.

0

jak kolega wyzej - konstruktor jest do tego aby skonstruowac poprawna instancje klasy. do wykonywania jakiejs uslugi dla zewnetrznej klasy uzyj dedykowanej metody

0

To nie zewnętrzna klasa. Cały kod:

public class FileLoader {

    private Set<Path> files;

    public FileLoader(){
        loadTextFiles();
    }

    private void loadTextFiles(){
        Path dir = Paths.get("/");
        try (DirectoryStream<Path> stream = Files.newDirectoryStream(dir, (Path p) -> p.endsWith(".txt"))){
            files = StreamSupport.stream(stream.spliterator(), false).collect(Collectors.toSet());
        }
        catch(IOException e){
            e.printStackTrace();
        }
    }
}

Osobna metoda po to, że a) przeczytasz nazwę i wiesz co robi, b) jeśli jest kilka takich metod to konstruktor byłby dłuugi, c) jeśli mam jakiś scope w konstruktorze (try w tym wypadku) i inicjalizuję zmienną final to nie muszę używać tymczasowych zmiennych (to akurat mniej ważne, ale też).

konstruktor jest do tego aby skonstruowac poprawna instancje klasy. do wykonywania jakiejs uslugi dla zewnetrznej klasy uzyj dedykowanej metody

@katelx to właśnie robię, konstruuję poprawną instancję klasy.

0

Tyle że ten konstruktor jest zbyt skomplikowany.
Plus istnieje możliwość, że rzuci wyjątek (który fakt, łapiesz, ale i tak), a to już w ogóle dysklasyfikuje metodę jako część konstruktora.

Zrób z tego loadTextFiles publiczną metodę zwracającą bool (czy wczytywanie się powiodło, zamiast na pałę e.printStackTrace();) i nie wywołuj jej z konstruktora.

0

@Wizzie no wlasnie nie. jak walnie ci wyjatek to bedzie to niepoprawna instancja klasy, czyz nie? to nie jest dobra praktyka zeby w konstruktorze uzywac i/o

0

Jako, że nie podoba mi się koncepcja obiektu, który do całkowitego skonstruowania potrzebuje wywoływania publicznej metody, wpadłem na taki pomysł (kod poniżej). W tym przypadku użycie klasy sprowadza się do stworzenia instancji i wywołania fileLoader.getFiles(). Co o tym myślicie?

public class FileLoader{
    private Set<Path> files = new HashSet<>();

    private boolean loadTextFiles(){
        Path dir = Paths.get("/");
        try (DirectoryStream<Path> stream = Files.newDirectoryStream(dir, (Path p) -> p.endsWith(".txt"))){
            files = StreamSupport.stream(stream.spliterator(), false).collect(Collectors.toSet());
            return true;
        }
        catch(IOException e){
            return false;
        }
    }

    public Set<Path> getFiles(){
        if(files.isEmpty()) {
            loadTextFiles();
        }
        return files;
    }
}
0

Taki lazy-loading już lepszy, choć nie wykorzystujesz wyniku zwracanego przez loadTextFiles.

Generalnie to loadTextFiles mogłoby równie dobrze być publiczną metodą statyczną zwracającą Set<Path> :P

1

spoko, tylko zastanawiaja mnie 2 rzeczy a) obsluga bledow b) co jak zostana dodane nowe pliki?

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