У меня есть простой код парсера на 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 ответ
Во-первых, было бы неплохо увидеть весь метод или, желательно, даже весь класс. Меньше контекста затрудняет обзор.
Тем не менее, я чувствую, что этого было достаточно для обзора, так что приступим.
Ты звонишь 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]);
}