Conversation
second try
|
|
||
| // Get the file and save it somewhere | ||
| byte[] bytes = file.getBytes(); | ||
| Path path = Paths.get(publishPath+"\\"+pathToUpload+'\\'+file.getOriginalFilename()); |
There was a problem hiding this comment.
Не во всех ОС разделителем является \. Сам метод Paths.get() за вас построит такую конкатенацию, если передать строковые параметры через запятую.
| // Get the file and save it somewhere | ||
| byte[] bytes = file.getBytes(); | ||
| Path path = Paths.get(publishPath+"\\"+pathToUpload+'\\'+file.getOriginalFilename()); | ||
| System.out.println(path ); |
There was a problem hiding this comment.
- Форматирование
- Вывод в консоль. Привыкайте пользоваться логгером! :-)
|
|
||
|
|
||
| } catch (IOException e) { | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
В серверных приложениях печать в консоль смысла не имеет. Используйте логгер
| HttpServletResponse response) { | ||
| try { | ||
| // get your file as InputStream | ||
| InputStream is = new FileInputStream( publishPath+"/"+fileName ); |
There was a problem hiding this comment.
А не нужно ли is закрыть по завершении?
| import java.util.List; | ||
|
|
||
| public class DirectoryContentsRefactorer { | ||
| public DirectoryContents refactor(List<FileData> fileDataList, DirectoryContents directoryContents){ |
There was a problem hiding this comment.
- Форматирование (Ctrl-Alt-Shift-L или Ctrl-Alt-F в раскладке Netbeans)
- Из названия совершенно неясно, какую проблему решает этот класс/метод. Возможно, Javadoc бы помог.
| package ru.amm.fileexplorer.server.entity; | ||
|
|
||
| public enum SortMethod { | ||
| DATE,NAME,SIZE,DEFAULT; |
There was a problem hiding this comment.
Обычно принято рассматривать два аспекта:
- Sort by (по чему сортируем)
- Sort direction: ascending, descending
| return new DirectoryContents(relativePath, parentDir, filteredList); | ||
| } | ||
| private List<FileData> getSortedlist(List<FileData> fileList, SortMethod sortMethod){ | ||
| switch (sortMethod){ |
There was a problem hiding this comment.
Switch в большинстве случаев - плохая идея. Его принято заменять либо паттерном стратегия, либо (хотя бы) поиском по Map<>. Почему бы не пойти путем, как сделан FileMatcher?
| dirContents = explorerService.getRootContents(SortMethod.valueOf( sortMethod )); | ||
| } else { | ||
| dirContents = explorerService.getContents(path); | ||
| dirContents = explorerService.getContents(path,SortMethod.valueOf( sortMethod )); |
There was a problem hiding this comment.
Вот такой вариант имхо проще и элегантнее (ваша версия осталась в комментах)
@RequestMapping(path = "/directories", method = RequestMethod.GET)
public ModelAndView index(@RequestParam(name = "path", required = false) String path, @RequestParam(name="sort",required = false) Optional<SortMethod> aSortMethod) {
DirectoryContents dirContents;
SortMethod sortMethod = aSortMethod.orElse(SortMethod.DEFAULT);
// if(sortMethod==null)sortMethod="DEFAULT";
if (path == null|| path.contains( "../" )||path.contains( "/.." )) {
// dirContents = explorerService.getRootContents(SortMethod.valueOf( sortMethod ));
dirContents = explorerService.getRootContents(sortMethod);
} else {
dirContents = explorerService.getContents(path, sortMethod);
}
Map<String, Object> data = new HashMap<>();
data.put("directory", dirContents);
return new ModelAndView("index", data);
}Иными словами, sortMethod в контроллере не обязан быть строкой, нам не надо руками его парсить через valueOf. Ну и Optional немного упрощает жизнь.
| dirContents = explorerService.getContents(path,sortMethod ); | ||
| } | ||
| FileMatcher fileMatcher = new NameMatcher(pattern); | ||
| List<FileData> filteredList= dirContents.getFiles().stream().filter( file->fileMatcher.matches(file)).collect( Collectors.toList()); |
There was a problem hiding this comment.
Слишком много логики для контроллера. Все сортировки надо уносить в сервисы.
|
|
||
| // Get the file and save it somewhere | ||
| byte[] bytes = file.getBytes(); | ||
| Path path = Paths.get(publishPath+"\\"+pathToUpload+'\\'+file.getOriginalFilename()); |
| e.printStackTrace(); | ||
| } | ||
|
|
||
| return "redirect:/directories/?path="+ pathToUpload.replace("\\" ,"%5C" )+"&sort=DEFAULT"; |
There was a problem hiding this comment.
Здесь очень легко забыть что-то еще заэкранировать (я про .replace). Лучше и проще передать параметр через RedirectAttributes
| try { | ||
|
|
||
| // Get the file and save it somewhere | ||
| byte[] bytes = file.getBytes(); |
There was a problem hiding this comment.
Поскольку byte[] это не ленивая структура, то все содержимое файла будет вычитано в память целиком. Это потенциальный OutOfMemory
|
|
||
| import java.util.List; | ||
|
|
||
| public class DirectoryContentsRefactorer { |
There was a problem hiding this comment.
Добавьте Javadoc или выберите более подходящее название. Я не понимаю, что делает этот класс.
| fileData.setDirectory(attr.isDirectory()); | ||
| String mimeType=Files.probeContentType( file ); | ||
| System.out.println(mimeType ); | ||
| if(mimeType==null)mimeType="directory"; |
There was a problem hiding this comment.
Вы уверены, что это однозначный критерий?
| @@ -0,0 +1,5 @@ | |||
| package ru.amm.fileexplorer.server.entity; | |||
|
|
|||
| public enum FileType { | |||
There was a problem hiding this comment.
Мне нравится идея отдавать в модель осмысленный тип файла, а не строку с mime-type.
| HttpServletResponse response) { | ||
| try { | ||
| // get your file as InputStream | ||
| InputStream is = new FileInputStream( publishPath+"/"+fileName ); |
There was a problem hiding this comment.
InputStream нигде не закрывается
| return new DirectoryContents(relativePath, parentDir, filteredList); | ||
| } | ||
| private List<FileData> getSortedlist(List<FileData> fileList, SortMethod sortMethod){ | ||
| switch (sortMethod){ |
There was a problem hiding this comment.
Как правило, switch почти всегда это плохая практика. Почему бы не пойти тем же путем, что и FileMatcher?
typeundefined
left a comment
typeundefined
left a comment
There was a problem hiding this comment.
Неплохая работа, но есть несколько моментов, которые нужно поправить (см. мои комментарии).
Основные претензии:
- Форматирование - просто не забывайте нажимать Ctrl-Alt-L (или Alt-Shift-F в Netbeans keymap). Иначе читается тяжело.
- Утечки ресурсов (память, незакрытый input stream)
…есто или кроме mime-type и как поступать с файлами без расширения, так же не знаю как именно избежать использования switch case(кроме создания матчера для каждого типа файла, что скорее всего будет выглядеть не очень красиво)
Rainboom7
left a comment
Rainboom7
left a comment
There was a problem hiding this comment.
попытка пофиксить большинство проблем,(кроме использования mimetype и swithc case)
2 participants
second try to create pull request properly