Как я могу переписать приведенный ниже код парсера на Java

У меня есть простой код парсера на java для учебного проекта. Может ли кто-нибудь проверить, как я могу улучшить код ниже

Потому что последние несколько дней я думал о том, как улучшить этот код, но ничего не получил

int counter = 0;
int cacheCounter = 0;



for(Map.Entry<String, File[]> path : queriedCache.entrySet()) {
        String[] array = new String[path.getValue().length];
        for(File file : queriedCache.get(path.getKey())) {
            if(file.listFiles() != null) {
                array = new String[Objects.requireNonNull(file.listFiles()).length + 1];
                for(File gif : Objects.requireNonNull(file.listFiles())) {
                    array[counter] = gif.getAbsolutePath();
                    counter++;
                }
                counter = 0;
            } else { 
                array[counter] = file.getAbsolutePath();
            }
            counter++;
        }
        counter = 0;
        cache[cacheCounter] = new CacheDto();
        cache[cacheCounter].setQuery(path.getKey());
        cache[cacheCounter].setGifs(array);
        cacheCounter++;
    }

    return cache;

1 ответ
1

Во-первых, было бы неплохо увидеть весь метод или, желательно, даже весь класс. Меньше контекста затрудняет обзор.

Тем не менее, я чувствую, что этого было достаточно для обзора, так что приступим.

Ты звонишь file.listFiles 3 раза на файл. Вероятно, было бы более эффективно сохранить результат в переменной при первом вызове, чтобы вам не приходилось делать это снова.

На этой ноте оба ваших Objects.requireNonNull звонки избыточны — они в ветке, где вы уже проверили file.listFiles() не null.

Поведение вашего кода кажется странный при работе с путем, содержащим как каталоги, так и файлы. В лучшем случае возвращаемое значение будет странным и / или непоследовательным. В худшем случае вы можете получить ArrayIndexOutOfBoundsException. Чтобы увидеть, как это сделать, давайте посмотрим на каталог со следующим содержимым:

some_file.gif
some_folder/
    cat.gif
    dog.gif
thing.gif
thing2.gif
thing3.gif
thing4.gif

Теперь мы не знать какой заказ listFiles() вернет эти файлы. Но давайте предположим, что они будут возвращены в том порядке, в котором я их указывал выше. В этом случае ваш код будет:

  • Создать массив array длиной 6
  • Перебрать содержимое этой папки:
    • Добавлять some_file.gif к array
    • Заметь some_folder — это каталог и перебрать его:
      • Заменять array с новым массивом длиной 3. some_file.gif теряется в процессе, что не идеально
      • Добавлять some_folder/cat.gif а также some_folder/dog.gif к этому массиву
      • Сброс настроек counter до 0
    • Добавлять thing.gif к array при индексе 0, перезапись some_folder/cat.gif — не уверен, что мы этого хотим
    • Добавлять thing2.gif к array по индексу 1, перезапись some_folder/dog.gif тоже
    • Добавлять thing3.gif к array в индексе 2. К счастью, array в нем 3 места, поэтому он подходит
    • Добавлять thing4.gif к array по индексу 3. array не имеет индекса 3, поэтому мы получаем ArrayIndexOutOfBoundsException

Я думаю, что вы хотите использовать какой-то динамический размер Collection<String> собирать пути к файлам. Намного проще, чем вести учет счетчиков. То же самое для cache. Если вам действительно нужны массивы в конце, коллекции имеют toArray метод.

Когда у вас есть Map.EntrySet, вы уже знаете, какие значения связаны с каждым ключом. Если вы не обновляете карту во время работы (а это не так), someMap.get(entry.getKey()) просто более сложный способ сказать entry.getValue()

И, как указал г-н Р. в комментарии, вы проверяете только один уровень каталогов. Если у вас есть такая структура:

one/
    two/
        three/
            hello.gif

… ты вернешься one/two, но я думаю, ты захочешь вернуться one/two/three/hello.gif. Если вы хотите проверить произвольную глубину, вы можете изучить другой подход для обхода файловой системы.

Собрав все вместе, я мог бы сделать что-нибудь вроде

public static CacheDto[] makeDTOs(Map<String, File[]> queriedCache) {
    Collection<CacheDto> cache = new ArrayList<>();

    for (Map.Entry<String, File[]> entry : queriedCache.entrySet()) {
        Collection<String> paths = new ArrayList<>();
        Queue<File> filesToCheck = new PriorityQueue<>(Arrays.asList(entry.getValue()));

        while ( ! filesToCheck.isEmpty() ) {
            File file = filesToCheck.poll();
            if (file.isDirectory()) {
                for (File child : file.listFiles()) {
                    filesToCheck.add(child);
                }
            } else {
                files.add(file.getAbsolutePath());
            }
        }

        CacheDto dto = new CacheDto();
        dto.setQuery(entry.getKey());
        dto.setGifs(paths.toArray(new String[0]));

        cache.add(dto);
    }

    return cache.toArray(new CacheDto[0]);
}

    Добавить комментарий

    Ваш адрес email не будет опубликован. Обязательные поля помечены *