Leonid ecobike #1
Conversation
| @@ -0,0 +1,170 @@ | |||
| package com.ecobike.dao; | |||
|
|
|||
| import static com.ecobike.EcoBikeApplication.COMMUNICATOR; | |||
There was a problem hiding this comment.
Я б рекомендував уникати статики. Краще реалізувати тут DI. Можна той же об'єкт communicator передати в якості параметру в конструкторі FileBikeDao
| */ | ||
| private Bike parseBike(String line) { | ||
| try { | ||
| if (line.startsWith(BikeType.SPEEDELEC.toString())) { |
There was a problem hiding this comment.
Що буде коли в тебе додасться ще один тип велосипедів? Швидше за все ти тоді будеш порушувати OCP із SOLID. Подумай, як можна було б розширяти функціонал без внесення змін в цей метод
There was a problem hiding this comment.
Добавил пакет parser, в нем отдельные классы-парсеры для каждого типа байков и в абстрактном парсере статический фабричный метод, который возвращает нужный парсер в зависимости от переданной строки
| synchronized (this) { | ||
| Stream<Bike> bikeStream = bikes.parallelStream() | ||
| .filter(bike -> bike.getBikeType() == parCont.getBikeType() | ||
| && bike.getBrand().equalsIgnoreCase(parCont.getBrand()) |
There was a problem hiding this comment.
А якщо я не вкажу brand при пошуку? Я отримаю велосипеди всіх брендів? Чи я отримаю велосипеди, де бренд не вказано?
There was a problem hiding this comment.
В сообветствии с заданием Тип и Бренд нужно указать обязательно, остальное опционально. Прилажуха так и работает - будет требовать Бренд до посинеия)
Option 5
Implement the search function, which allows you to find the products of a certain type (ebikes, folding bikes and speedelecs) of a given brand and with the given parameters. The list of
parameters can be any from empty (then the search works only by the name of the brand) to the full
(when all parameters specific to this type of bike are set).
| } | ||
|
|
||
| /** | ||
| * Method reads text from file and loads parsed Bike objects to DataHolder. |
There was a problem hiding this comment.
слово and вказує на те, що порушується принцип SRP. Постарайся відрефакторити цей метод
There was a problem hiding this comment.
Отрефакторил BikeDao, теперь он ничего не знает о дата холдере, а просто читает данные и возвращает лист байков в методе readBikes и записывает лист байков в методе saveBikes
| * operation specified by user. | ||
| */ | ||
| public interface Command { | ||
| Communicator COMMUNICATOR = EcoBikeApplication.COMMUNICATOR; |
There was a problem hiding this comment.
В тебе в реалізації майже всюди порушується DI. Спробуй відрефакторити своє рішення.
| */ | ||
| public synchronized void init() throws IllegalDataSourceException { | ||
| bikes.clear(); | ||
| bikes.addAll(bikeDao.readBikes()); |
There was a problem hiding this comment.
Не до кіняці можу зрозуміти. Де ми зберігаємо bikes? Виглядає, що ти збергіаєш bikes тут, в класі DataHolder, але при цьому зберігаєш їх ще десь, звідки ти отримуєш за допомогою DAO.
There was a problem hiding this comment.
Так у нас по заданию:
- Reads in the file ecobike.txt when starting the program
Вот я в начале работы программы считываю данные из файла и загружаю в DataHolder, этот класс хранит их во время рантайма. Поиск или добавление новых байко мы делаем в DataHolder. Если пользователь хочет, то записывает обновленные данные в файл.
| } | ||
| synchronized (this) { | ||
| Stream<Bike> bikeStream = bikes.parallelStream() | ||
| .filter(bike -> |
There was a problem hiding this comment.
@Rommelua Як ти дивишся на те, щоб реалізувати щось типу FilterChain?
Зараз я бачу вузьке місце - мені щоб додати новий фільтр - треба редагувати ось цей метод. Як на мене - це порушення OCP.
FilterChain дозволяє тобі динамічно додавати фільтри. Глянь наприклад як працюють фільтри по авторизації.
Дуже просто опишу як я це бачу:
if (attributes == null || attributes.isEmpty()) {
// skip this filter
return;
}
Якщо параметри для фільтру присутні - фільтруєш список велосипедів.
Приклад фільтру: BrandFilter, WeightFilter, ColorFilter, etc...
| expectedEx.expect(IllegalArgumentException.class); | ||
| bikeParser = FileBikeParser.getBikeParser(WRONG); | ||
| } | ||
| } |
There was a problem hiding this comment.
checkstile, очевидно, не проверяет тесты, погуглю...
No description provided.