Первая реализация ТЗ 7ого спринта. #2
Conversation
EugeneLenkevich
left a comment
EugeneLenkevich
left a comment
There was a problem hiding this comment.
Привет, Вячеслав, спасибо за присланное решение
В целом все верно, но посмотри на читаемость кода, есть моментики которые ухудшат его понятность. Плюс тест, не настаиваю, но если расширишь его разными примерами фалов, то будет полезно :)
| public Task fromString(String value) { | ||
| String[] mass = value.split(","); | ||
|
|
||
| if (mass[1].equals("TASK")) { |
There was a problem hiding this comment.
mass[1]
такие бывает трудновато читать, а давай заведем отдельны переменные с говорящими именами, присвоим им значения соответствующих элементов массива и будем их использовать ниже :)
| } | ||
| } | ||
| } catch (IOException e) { | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
А давай тут кидать, например, ManagerLoadException, исключение аналогичное тому что кидаем при сохранении :)
| try { | ||
| List<String> lines = Files.readAllLines(file.toPath()); | ||
| for (String string : lines) { | ||
| if (!string.equals(lines.get(0))) { |
There was a problem hiding this comment.
В данном случае не проще ли было использовать цикл со счетчиком чтобы сразу начать со второго элемента, если я правильно этот код понимаю :)
И имя переменной string слишком общее, пусть например row будет
| } | ||
| assertEquals(newFile.size(), 7); | ||
|
|
||
| } catch (IOException e) { |
There was a problem hiding this comment.
В тесте можно не перехватывать исключение, junit и так распечатает стректейс ошибки :)
There was a problem hiding this comment.
А какую ошибку, предложит в сигнатуру метода прописать? давай сделаем
| void loadFromFile() { | ||
| FileBackedTaskManager.loadFromFile(new File("test/manager/TestFile.csv")); | ||
| try { | ||
| List<String> newFile = Files.readAllLines(Paths.get("src/file.csv")); |
There was a problem hiding this comment.
Верхний тест вроде бы то де самое проверяет, то есть и сохранение и загрузку :)
Но пусть будет, но тогда еще можно тут проверять загрузку разных примеров файлов. Допустим добавить еще пример пустого файла, файла с ошибкой и пр
То есть в общем случае мы тестируем
- happy path, как сейчас - файл корректен, записи есть
- ошибка, файл некорректен, можно причем проверить поведение менеджера на разных ситуациях, разные ошибки в данных
- граничные условия, файл пуст, например
EugeneLenkevich
left a comment
EugeneLenkevich
left a comment
There was a problem hiding this comment.
Привет! Давай все таки добьем тесты, уберем там try-catch, если предлагает исключение прописать в сигнатуру метода то это вполне себе допустимо :)
| } | ||
| assertEquals(newFile.size(), 7); | ||
|
|
||
| } catch (IOException e) { |
There was a problem hiding this comment.
А какую ошибку, предложит в сигнатуру метода прописать? давай сделаем
No description provided.