HW4 #5
HW4 #5
Conversation
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
Здесь куча файлов, котоыре не надо выкладывать. Исходники Trie ещё ладно, но бинарники и куча лишних файлов от ZipFile в гите точно не нужны.
Зато нужны юнит-тесты
| /** | ||
| * Custom exception to throw if folder was not found | ||
| */ | ||
| class FolderNotFoundException extends Exception { |
There was a problem hiding this comment.
Его надо сделать public, иначе его будет проблематично ловить вне пакета
| /** | ||
| * Custom exception to throw in case of zip file problems | ||
| */ | ||
| class ZipException extends Exception { |
| * scans all archives in directory, unpacks only files that match the regex | ||
| * @throws FolderNotFoundException | ||
| * @throws FileNotFoundException | ||
| * @throws ZipException |
There was a problem hiding this comment.
Пустые комментарии --- это плохо
|
|
||
| BufferedInputStream fileInputStream = new BufferedInputStream(new FileInputStream(fileEntry)); | ||
|
|
||
| ZipInputStream zipInputStream = new ZipInputStream(fileInputStream); |
There was a problem hiding this comment.
Стримы надо не забывать закрывать. Причём, для этого есть try-with-resources
| zipEntry = zipInputStream.getNextEntry(); | ||
| } | ||
| } | ||
| } catch (NullPointerException e) { |
There was a problem hiding this comment.
NullPointerException ловить нельзя :)
| } catch (NullPointerException e) { | ||
| throw new FolderNotFoundException(); | ||
| } catch (IOException e) { | ||
| throw new ZipException(); |
There was a problem hiding this comment.
Так теряется информация об исключении, которая была в e. Хороший пример антипаттерна Error Hiding (https://en.wikipedia.org/wiki/Error_hiding)
| fileOutputStream.write(bytes, 0, length); | ||
| } | ||
| inputStream.close(); | ||
| fileOutputStream.close(); |
There was a problem hiding this comment.
Тут тоже лучше try-with-resources, этот код беззащитен перед исключениями
|
|
||
| public static void main(String[] args) { | ||
| String path = args[0]; | ||
| String regex = args[1]; |
There was a problem hiding this comment.
Надо проверять пользовательский ввод, людям свойственно ошибаться
| @@ -0,0 +1,124 @@ | |||
| import java.io.*; | |||
There was a problem hiding this comment.
Класс вне пакета --- это плохо
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
Ну как-то не до конца почищено. .gradle же осталась, и workspace.xml. И всё-таки нужны юнит-тесты
| @@ -0,0 +1,128 @@ | |||
| package ZipFiler; | |||
There was a problem hiding this comment.
Пакет неканонично назван
| /** | ||
| * Custom exception to throw if folder was not found | ||
| */ | ||
| public class FolderNotFoundException extends Exception { |
| /** | ||
| * Custom exception to throw in case of zip file problems | ||
| */ | ||
| public class ZipException extends Exception { |
| private void writeEntry(ZipFile zipFile, ZipEntry zipEntry) { | ||
| try (InputStream inputStream = zipFile.getInputStream(zipEntry); | ||
| FileOutputStream fileOutputStream = new FileOutputStream(zipEntry.getName()); | ||
| ){ |
There was a problem hiding this comment.
Надо пробел перед "{"
No description provided.