Visualizzazione dei risultati da 1 a 9 su 9

Discussione: Migliorare metodo per creazione lista immagini

  1. #1

    Migliorare metodo per creazione lista immagini

    ciao!

    cercando di migliorare un pò il "design" di un programmino, volevo un consiglio sul codice che avevo scritto.

    questo quello iniziale:
    codice:
    public class ListImages {
        public static ArrayList<String> getImages(Stage stage) {
            ArrayList<String> images = new ArrayList<>();
            DirectoryChooser dc = new DirectoryChooser();
            dc.setTitle("Open photo directory");
            dc.setInitialDirectory(new File(System.getProperty("user.home")));
            File dir = dc.showDialog(stage);
            if (dir != null) {
                File[] files = dir.listFiles(
                        (dir1, name) -> (
                                name.endsWith(".jpg") ||
                                        name.endsWith(".jpeg")
                                        || name.endsWith(".png")
                        ));
                for (File f : files) {
                    images.add(f.getAbsolutePath());
                }
            }
            return images;
        }
    }
    l'avrei spezzato in due classi separate.
    questo fa partire solo il DirectoryChooser:
    codice:
    public class ListImages {
        public static File chooseDir(Stage stage) {
            DirectoryChooser dc = new DirectoryChooser();
            dc.setTitle("Open photo directory");
            dc.setInitialDirectory(new File(System.getProperty("user.home")));
            return dc.showDialog(stage);
        }
    }
    questo prende la directory e lista le immagini:
    codice:
    public class ListImagesDirectory {
        public ArrayList<String> getImages(File dir) {
            ArrayList<String> images = new ArrayList<>();
            if (dir != null) {
                File[] files = dir.listFiles(
                        (dir1, name) -> (
                                name.endsWith(".jpg") || name.endsWith(".jpeg") || name.endsWith(".png")
                        ));
                for (File f : files) {
                    images.add(f.getAbsolutePath());
                }
            }
            return images;
        }
    }
    secondo può andare?
    o era inutile fare questa modifica??

  2. #2
    Quote Originariamente inviata da fermat Visualizza il messaggio
    l'avrei spezzato in due classi separate.
    questo fa partire solo il DirectoryChooser:
    [...]
    questo prende la directory e lista le immagini:
    [...]
    secondo può andare?
    o era inutile fare questa modifica??
    Tecnicamente può anche andare. Stai però dimenticando un aspetto un po' più "pratico". Ogni volta che l'utente apre la dialog si ritrova nella home directory e magari deve fare parecchi click/azioni per spostarsi dove vuole. E se lo deve fare svariate volte .... è una rottura.
    Tutto questo fa pensare di non fare questa logica in un contesto static ma di "promuovere" tutto quanto ad una classe per cui l'oggetto possa mantenere l'ultima directory scelta. Chiaramente poi l'oggetto va tenuto in vita per la durata della applicazione.

    Tradotto in codice, ecco cosa farei io:

    codice:
    public class DirChooser {
        private File lastDir;
    
        public DirChooser() {
            lastDir = new File(System.getProperty("user.home"));
        }
    
        public Optional<File> showDialog(Window ownerWindow, String title) {
            DirectoryChooser chooser = new DirectoryChooser();
            chooser.setTitle(title);
            chooser.setInitialDirectory(lastDir);
    
            File dir = chooser.showDialog(ownerWindow);
    
            if (dir != null) {
                lastDir = dir;
                return Optional.of(dir);
            } else {
                return Optional.empty();
            }
        }
    }

    È ovviamente un po' più lunga del tuo chooseDir ma ha diversi vantaggi e aspetti positivi.

    1) Non è specifica solo per immagini ma vale in generale. Cioè è riutilizzabile anche per altro.
    2) L'oggetto DirChooser, se tenuto a lungo in vita per più showDialog, mantiene l'ultima directory scelta.
    3) Nota come ho usato Optional (da Java 8) che permette di evitare di dover gestire un null nel chiamante.

    Nota anche che showDialog di DirChooser riceve Window (javafx.stage.Window) e non Stage. Il showDialog di DirectoryChooser riceve Window ed io ho mantenuto la stessa "ampiezza" senza restringere nulla. Si può anche aggiungere un overload senza Window che chiama l'altro con null.
    Andrea, www.andbin.net – Senior Java developer – SCJP 5 (91%) – SCWCD 5 (94%)
    Il mio blog sulla programmazione

  3. #3
    si infatti volevo sistemare per rendere il DirectoryChooser riusabile.
    ma non avevo pensato a questa finezza in effetti.

    ho preso il tuo codice, e poi fatto questo nel controller:
    codice:
    public class MainController {
    
        private Stage stage = null;
        private ListImagesDirectory lid = null;
        private DirChooser dc;
    
        @FXML
        public void initialize() {
            dc = new DirChooser();
        }
    
        private Stage getStage() {
            stage = (Stage) mainPane.getScene().getWindow();
            return stage;
        }
    
        @FXML
        private void openDirectory() {
            lid = new ListImagesDirectory();
            images = lid.getImages(dc.showDialog(getStage().getOwner(), "Choose directory").get());
        }
    
    }
    dove ListImagesDirectory l'ho postato sopra.
    sembra funzionare alla perfezione!!

    grazie mille per la dritta!!

  4. #4
    E per il FilenameFilter farei una classe apposita per il filtro espressamente sulle estensioni. Anche questa sarebbe generica e ben riutilizzabile:

    codice:
    public class ExtensionFilenameFilter implements FilenameFilter {
        private final String[] extensions;
    
        public ExtensionFilenameFilter(String... extensions) {
            this.extensions = Arrays.stream(extensions).map(String::toLowerCase).toArray(String[]::new);
        }
    
        @Override
        public boolean accept(File dir, String name) {
            int i = name.lastIndexOf('.');
    
            if (i >= 0) {
                String fileExt = name.substring(i+1).toLowerCase();
    
                for (String extension : extensions) {
                    if (fileExt.equals(extension)) {
                        return true;
                    }
                }
            }
    
            return false;
        }
    }

    P.S. se puoi usare la Apache Commons IO verrebbe un pelino meglio grazie al getExtension(String) della FilenameUtils.
    Ultima modifica di andbin; 12-02-2018 a 12:31
    Andrea, www.andbin.net – Senior Java developer – SCJP 5 (91%) – SCWCD 5 (94%)
    Il mio blog sulla programmazione

  5. #5
    mitico!!

    allora, apache commons io già la uso.
    quindi ho modificato così:
    codice:
    import org.apache.commons.io.FilenameUtils;
    
    import java.io.File;
    import java.io.FilenameFilter;
    import java.util.Arrays;
    
    public class ExtensionFilenameFilter implements FilenameFilter {
    
        private final String[] extensions;
    
        public ExtensionFilenameFilter(String... extensions) {
            this.extensions = Arrays.stream(extensions).map(String::toLowerCase).toArray(String[]::new);
        }
    
        @Override
    public boolean accept(File dir, String name) {
            String fileExt = FilenameUtils.getExtension(name.toLowerCase());
            for (String extension : extensions) {
                if (fileExt.equals(extension)) {
                    return true;
                }
            }
    
            return false;
        }
    }
    poi ho modificato il metodo che lista i files:
    codice:
    import java.io.File;
    import java.util.ArrayList;
    
    public class ListFilesDirectory {
    
        public ArrayList<String> getFilesExts(File dir, String... extensions) {
            ArrayList<String> listfiles = new ArrayList<>();
            if (dir != null) {
                File[] files = dir.listFiles(new ExtensionFilenameFilter(extensions));
                for (File f : files) {
                    listfiles.add(f.getAbsolutePath());
                }
            }
            return listfiles;
        }
    }
    così anche questo dovrebbe essere più generico.
    poi a cascata nel controller:
    codice:
    lid = new ListFilesDirectory();
    String[] exts = {"jpg", "jpeg", "png"};
    images = lid.getFilesExts(dc.showDialog(getStage().getOwner(), "Choose directory").get(), exts);
    funzionare sembra funzionare bene!

  6. #6
    Occhio solo ad alcuni piccoli aspetti da tenere presente:

    Quote Originariamente inviata da fermat Visualizza il messaggio
    codice:
    images = lid.getImages(dc.showDialog(getStage().getOwner(), "Choose directory").get());
    Il get() non va usato così in modo indiscriminato. Perché se il Optional è "vuoto", il get() lancia NoSuchElementException.
    Il Optional, detto in generale, andrebbe gestito o usando es. isPresent() (cioè testando prima di fare il get() ), oppure usandolo in maniera più "funzionale" con i map/filter/ecc...


    Quote Originariamente inviata da fermat Visualizza il messaggio
    codice:
        public ArrayList<String> getFilesExts(File dir, String... extensions) {
            ArrayList<String> listfiles = new ArrayList<>();
            if (dir != null) {
                File[] files = dir.listFiles(new ExtensionFilenameFilter(extensions));
                for (File f : files) {
                    listfiles.add(f.getAbsolutePath());
                }
            }
            return listfiles;
        }

    Attenzione ad un piccolo cavillo. Il listFiles può restituire null in due casi: il File usato non è una directory o c'è stato un errore di I/O durante la scansione. Questo andrebbe considerato. Nel tuo caso sbucherebbe fuori un NullPointerException all'inizio del for-each.


    P.S.
    Riguardo il ExtensionFilenameFilter c'è una cosa che non mi è venuta in mente prima. Sono partito in quarta per farti vedere come implementavo FilenameFilter. Ma visto che si tratta di filtrare solo file, sarebbe stato più utile implementare FileFilter per poter testare facilmente se è un file con isFile().
    E' una finezza nel senso che difficilmente qualcuno si fa una directory chiamata qualcosa.jpg ... ma sai, non si sa mai ....
    Andrea, www.andbin.net – Senior Java developer – SCJP 5 (91%) – SCWCD 5 (94%)
    Il mio blog sulla programmazione

  7. #7
    allora.

    per quanto riguarda il primopunto, in effetti se non veniva scelto nulla, veniva generata una eccezione.
    nn lo avevo considerato.
    ho modificato così:
    codice:
    lid = new ListFilesDirectory();
    String[] exts = {"jpg", "jpeg", "png"};
    Optional<File> optFile = dc.showDialog(getStage().getOwner(), "Choose directory");
    if (optFile.isPresent()) {
        images = lid.getFilesExts(optFile.get(), exts);
        // ECCETERA
    }
    in questo modo, se viene cliccato annulla sul directorychooser, non succede nulla.

    per la seconda questione.
    in effetti avevo pensato di fare un controllo per verificare sia una directory.
    ma avendo scelto un DirectoryChooser, ho pensato fosse inutile, visto che si possono selezionare solo directory.
    ma in effetti, potrei riusare il tutto anche in altre circostanze, e quindi metterò un controllo anche su questo.

    per l'ultimo punto, intendi una cosa del genere??
    codice:
    public class ExtensionFilenameFilter implements FileFilter {
    
        private final String[] extensions;
    
        public ExtensionFilenameFilter(String... extensions) {
            this.extensions = Arrays.stream(extensions).map(String::toLowerCase).toArray(String[]::new);
        }
    
        @Override
    public boolean accept(File file) {
            if (file.isFile()) {
                String fileExt = FilenameUtils.getExtension(file.getName().toLowerCase());
                for (String extension : extensions) {
                    if (fileExt.equals(extension)) {
                        return true;
                    }
                }
            }
            return false;
        }
    }

  8. #8
    Quote Originariamente inviata da fermat Visualizza il messaggio
    in questo modo, se viene cliccato annulla sul directorychooser, non succede nulla.
    Corretto.

    Quote Originariamente inviata da fermat Visualizza il messaggio
    per la seconda questione.
    in effetti avevo pensato di fare un controllo per verificare sia una directory.
    ma avendo scelto un DirectoryChooser, ho pensato fosse inutile, visto che si possono selezionare solo directory.
    ma in effetti, potrei riusare il tutto anche in altre circostanze, e quindi metterò un controllo anche su questo.
    Sul fatto che sia una directory possiamo stare tranquilli, cioè è ragionevole pensare che il DirectoryChooser di JavaFX permetta di scegliere e ci fornisca solo delle directory e non faccia stupidate (se le facesse, sarebbe un suo baco).
    Ma i list/listFiles di File possono dare null anche in caso di un qualche errore di I/O durante la scansione della directory. E questo va considerato, in generale.

    Quote Originariamente inviata da fermat Visualizza il messaggio
    per l'ultimo punto, intendi una cosa del genere??
    Esattamente. Così se qualcuno un po' "sbadato" creasse una directory es. prove.jpg perlomeno la scarti, perché non consideri solo il nome e basta ma anche il tipo di entry.
    Ultima modifica di andbin; 12-02-2018 a 19:07
    Andrea, www.andbin.net – Senior Java developer – SCJP 5 (91%) – SCWCD 5 (94%)
    Il mio blog sulla programmazione

  9. #9
    perfetto, mitico come al solito!!
    grazie mille!!

Permessi di invio

  • Non puoi inserire discussioni
  • Non puoi inserire repliche
  • Non puoi inserire allegati
  • Non puoi modificare i tuoi messaggi
  •  
Powered by vBulletin® Version 4.2.1
Copyright © 2018 vBulletin Solutions, Inc. All rights reserved.