Гость
Войти | Профиль | Очистить
Целевая тема:
Создать новую тему:
Автор:
Действия ...
Форумы / Java [игнор отключен] [закрыт для гостей] / Рефакторинг/Применение паттернов / 12 сообщений из 12, страница 1 из 1
05.09.2017, 20:34
    #39516006
archelite
archelite
Гость
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Рефакторинг/Применение паттернов
Здравствуйте форумчане. Сначала опишу задачу

нужно написать метод, который принимает на вход две коллекции(не уточняется какие) объектов и возвращает объединенную по определенным правилам коллекцию с этими же объектами.

Объект - это цена на товар, имеет следующие поля

long id; // идентификатор в БД
String product_code; // код товара
int number; // номер цены
int depart; // номер отдела
Date begin; // начало действия
Date end; // конец действия
long value; // значение цены в копейках

объединять нужно с учетом таких условий

у товара с одним кодом может быть несколько номеров цены, товар с тем же кодом и номером цены может быть в разных отделах.
Но в одном отделе с одинаковым номер и кодом товара может быть только одна цена.

пример:
Product_code Number Depart Begin End valueИмеющиеся цены122856 1 1 01.01.2013 00:00:00 31.01.2013 23:59:59 11000122856 2 1 10.01.2013 00:00:00 20.01.2013 23:59:59 990006654 1 2 01.01.2013 00:00:00 31.01.2013 00:00:00 5000Новые цены122856 1 1 20.01.2013 00:00:00 20.02.2013 23:59:59 11000122856 2 1 15.01.2013 00:00:00 25.01.2013 23:59:59 920006654 1 2 12.01.2013 00:00:00 13.01.2013 00:00:00 4000Результат:122856 1 1 01.01.2013 00:00:00 20.02.2013 23:59:59 11000122856 2 1 10.01.2013 00:00:00 15.01.2013 00:00:00 99000122856 2 1 15.01.2013 00:00:00 25.01.2013 23:59:59 920006654 1 2 01.01.2013 00:00:00 12.01.2013 00:00:00 50006654 1 2 12.01.2013 00:00:00 13.01.2013 00:00:00 40006654 1 2 13.01.2013 00:00:00 31.01.2013 00:00:00 5000

Суть в том что данные в старом и новом списках естественно могут и будут несовпадать в отличие от примера.
В новом списке могут быть новые коды(добавляем в объединенный) или в новом отсутствуют старые(переносим в объединенный старые данные)

Я решил делать следующим образом:
1)берем из текущего списка первый объект, берем код продукта
2)получаем из текущего списка подмножество объектов с текущим кодом
3)берем объект уже из списка с текущим кодом, берем номер цены
4)подмножество с номером цены из подмножества с текущим объектом
5)из списка с номером - объект с отделом, номер отдела
6)подмножество с номером отдела (здесь в идеале должен быть 1 объект, как в списке текущих так и новых цен и уже конкретно эти объекты сравниваем между собой по датам и формируем новые объекты в соответствии в требованиями)

Если писать "в лоб" получается очень много похожего кода в котором легко запутаться, я вынес часть в отдельный метод, стало получше, но все равно не очень.

Код: java
1.
2.
3.
4.
5.
6.
7.
8.
9.
10.
11.
12.
13.
14.
15.
16.
17.
18.
19.
20.
21.
22.
23.
24.
25.
26.
27.
28.
29.
30.
31.
32.
33.
34.
35.
36.
37.
38.
39.
40.
41.
42.
43.
44.
45.
46.
47.
48.
49.
50.
    public List<Price> mergePriceLists(List<Price> curPrices, List<Price> newPrices) {
        
        List<Price>[] subLists;

        if (curPrices.size() - newPrices.size() >= 0) {
            for (int i = 0; i < curPrices.size(); i++) {
                subLists = subFilter(curPrices, newPrices, curPrices.get(i).product_code, "product_code");
                if (subLists[0].size() >= subLists[1].size()){
                    for (int j = 0; j < subLists[0].size(); j++){
                        subLists = subFilter(subLists[0], subLists[1], subLists[0].get(j).number, "number");
                        if (subLists[0].size() >= subLists[1].size()){
                            for (int k = 0; k < subLists[0].size(); k++){
                                subLists = subFilter(subLists[0], subLists[1], subLists[0].get(k).depart, "depart");
                                if (subLists[0].size() == 1 && subLists[1].size() == 1)
                                    mergePrices(subLists[0].get(0), subLists[1].get(0));  //метод который сравнивает объекты и формирует новые
                            }
                        } else {
                          //много кода
                        }
                    }
                   //.... еще куча else и в каждом много кода 
        return mergedPrices;
    }


    public List[] subFilter(List<Price> curPrices, List<Price> newPrices, Object compareParam, String paramName) {

        List<Price>[] subList = new List[2];

//        if (compareParam == null && paramName.trim().isEmpty()){
//            mergePrices(curPrices.get(0), newPrices.get(0));
//        } else
//            {
            if (compareParam instanceof String && paramName.equals("product_code")) {
                subList[0] = curPrices.stream().filter(s -> s.product_code.equals(compareParam)).collect(Collectors.toList());
                subList[1] = newPrices.stream().filter(s -> s.product_code.equals(compareParam)).collect(Collectors.toList());
            } else if (compareParam instanceof Integer) {
                switch (paramName) {
                    case "number":
                        subList[0] = curPrices.stream().filter(s -> s.number == (Integer) compareParam).collect(Collectors.toList());
                        subList[1] = newPrices.stream().filter(s -> s.number == (Integer) compareParam).collect(Collectors.toList());
                        break;
                    case "depart":
                        subList[0] = curPrices.stream().filter(s -> s.depart == (Integer) compareParam).collect(Collectors.toList());
                        subList[1] = newPrices.stream().filter(s -> s.depart == (Integer) compareParam).collect(Collectors.toList());
                        break;
                }
            }
        return subList;
    }



После того как подмножество обработано, чтобы не натыкаться на повторяющиеся данные я планирую удалять их из списков полученных на вход removeAll(текущее подмножество);

Таким образом в конце обработки входные коллекции должны оказаться пустыми.
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
06.09.2017, 07:34
    #39516114
Petro123
Petro123
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Рефакторинг/Применение паттернов
archelite,
На больших списках тормозит?
Варианты:
- в субд sql' ем
- паттерн Визитор.
?
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
06.09.2017, 09:14
    #39516150
Blazkowicz
Blazkowicz
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Рефакторинг/Применение паттернов
archelite,

Вы зачем-то намешали в свой вопрос кучу барахла из предметной области, которая тут никому не интересна. Отчего понять ваш вопрос становится сложно.

archeliteу товара с одним кодом может быть несколько номеров цены, товар с тем же кодом и номером цены может быть в разных отделах.
Но в одном отделе с одинаковым номер и кодом товара может быть только одна цена.
За такую формулировку надо эксперту предметной области чем-то тяжелым по голове бить. "Может быть", а "может и не быть" ничего не говорят о том что мы должны получить в результате. И что делать с дупликатами. И вообще больше вопросов чем ответов.

Абстрагируйтесь от предметной области. Есть два списка объектов. Нужно сформировать объединение этих списков, но по каким-то признакам объекты могут считаться идентичными их в списке быть не должно. Соответсвенно поля которые влияют на идентичность это в данном контексте - ключ объектов. Вам нужно разделить объект на ключ и остальные данные.

Для того чтобы получить уникальные объекты нужно поместить их в HashMap по ключу.
Если нужно как-то комбинировать объекты с идентичным ключом, для этого есть метод Map.merge(key, value, remappingFunction)
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
06.09.2017, 11:17
    #39516271
archelite
archelite
Гость
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Рефакторинг/Применение паттернов
Petro123,

авторНа больших списках тормозит?
- в субд sql' ем
- паттерн Визитор.

Смотря что считать большими списками
Это тестовое задание, так что нужно в коде)
Хз как его применить потому что точно неизвестно по каким критериям сравнивать объекты

Blazkowicz,

В целом вы пересказали мои мысли по этому вопросу, я тоже сначала подумал про мапу и ключи, но из за несколько размытого условия непонятного как это сделать. Есть еще метод retainAll но он сравнивает по equals'у а мне бы по компаратору, но в стандартном API я такого не нашел.

У меня есть решение для частного случая, когда каждому объекту есть одно соответствие, а решение для всех возможных случаев получается громоздкое. В обычном случае я бы просто уточнил условия, а это тестовая задача, уточнять как то немного неловко, хотя может они специально так сформулировали.
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
06.09.2017, 11:58
    #39516322
Blazkowicz
Blazkowicz
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Рефакторинг/Применение паттернов
archeliteВ целом вы пересказали мои мысли по этому вопросу

Телепаты в отпуске. Если не хотите пересказа своих мыслей, то стоило их опубликовать.

archelite я тоже сначала подумал про мапу и ключи, но из за несколько размытого условия непонятного как это сделать.

Фраза "Я решил делать следующим образом:" намекает на то что вам всё понятно. Вопроса "чтобы авторы задачи могли иметь ввиду?" я тоже не вижу.

archelite Есть еще метод retainAll но он сравнивает по equals'у а мне бы по компаратору, но в стандартном API я такого не нашел.
Направление мысли правильное. Проблема с вычислением equals/hashCode снаружи объекта легко решается новым объектом.

archeliteУ меня есть решение для частного случая
Это жопа какая-то а не решение. Копипаст на копипасте. Нолики и единички. Куча ни о чем не говорящих идентификаторов. Я уже не говорю про количество кода для решения примитивной задачи. Введение нового поля в ключ у вас приведёт к очередной копипасте кучи блоков кода?

archelite, когда каждому объекту есть одно соответствие
Что такое одно соответствие? Чем оно отличается от многих?

archelite, а решение для всех возможных случаев получается громоздкое.
Если ограничивать себя использованием if...else и массивами, то да. Но как-то JSE API пообширнее будет. Я в таком виде свою первую программу писал. Крестики-нолики. Меня хватило только на реализацию первого хода. Это была копипаста из 9 возможных вариантов. Если бы я и дальше писал в этом стиле, то на каждый из возможных состояний поля у меня бы была схожая секция кода. С тех пор я всегда переиспользую код (до тех пор пока копипаста не приводит к более простому коду, чем полноценная абстракция). И вам советую.

archeliteВ обычном случае я бы просто уточнил условия, а это тестовая задача, уточнять как то немного неловко, хотя может они специально так сформулировали.

Код: java
1.
2.
3.
4.
5.
6.
7.
8.
9.
10.
11.
class ProductPriceMergeKey {
    final Product product
    public ProductPriceMergeKey (Product product){...}
    //Вычисляем используя только нужные поля product
    int hashCode(){...}
    boolean equals(){...}
}

Map<Product> map = prices.stream().collect(toMap(p -> new ProductPriceMergeKey(p), p->p)); 

newPriceMap.forEach((k, p) -> oldPriceMap.merge(k, p, (newPrice, oldPrice) -> newPrice.merge(oldPrice));



Я бы ещё больше заморочился - ProductPriceMergeKey отделил бы от ProductPrice, а передавал туда только getter-ы через лямбды.

Код: java
1.
2.
3.
4.
5.
6.
7.
8.
class MergeKey<T>{
     public MergeKey(T instance, Supplier ... getters)
}

MergeKey<ProductPrice> key = new MergeKey(productPrice, 
   productPrice::getProductCode, 
   productPrice::getPriceID, 
   productPrice::getDepartmentID);
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
06.09.2017, 12:02
    #39516332
Blazkowicz
Blazkowicz
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Рефакторинг/Применение паттернов
Blazkowicz
Код: java
1.
2.
3.
4.
5.
6.
7.
8.
class MergeKey<T>{
     public MergeKey(T instance, Supplier ... getters)
}

MergeKey<ProductPrice> key = new MergeKey(productPrice, 
   productPrice::getProductCode, 
   productPrice::getPriceID, 
   productPrice::getDepartmentID);



instance тут не нужен по-идее. Мы его не явно через геттеры протащим. Просто иногда бывает нужно использовать Function вместо Supplier чтобы вызывать их для разных instance.
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
06.09.2017, 12:17
    #39516349
Никанор Кузьмич
Никанор Кузьмич
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Рефакторинг/Применение паттернов
archeliteОбъект - это цена на товар, имеет следующие поля

long id; // идентификатор в БД
String product_code; // код товара
int number; // номер цены
int depart; // номер отдела
Date begin; // начало действия
Date end; // конец действия
long value; // значение цены в копейках

объединять нужно с учетом таких условий

у товара с одним кодом может быть несколько номеров цены, товар с тем же кодом и номером цены может быть в разных отделах.
Но в одном отделе с одинаковым номер и кодом товара может быть только одна цена.Эх, Petro123 выше дело говорит, а вы его не слушаете...
Код: sql
1.
2.
3.
4.
5.
6.
select *
  from (select t.*, row_number() over (partition by depart, product_code order by date_begin desc) rn
          from (select * from old_price
                union all 
                select * from new_price) t)
         where rn = 1

Это для оракла, под вашу СУБД сами как-нибудь перепишете...
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
06.09.2017, 12:29
    #39516363
Blazkowicz
Blazkowicz
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Рефакторинг/Применение паттернов
archelite Сначала опишу задачу
нужно написать метод...
Классическая ошибка в постановке целей. У вас нет задачи написать метод. У вас есть задача выполнить тестовое задание.
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
06.09.2017, 17:40
    #39516676
archelite
archelite
Гость
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Рефакторинг/Применение паттернов
Blazkowicz,

авторclass ProductPriceMergeKey {
final Product product
public ProductPriceMergeKey (Product product){...}
//Вычисляем используя только нужные поля product
int hashCode(){...}
boolean equals(){...}

Map<Product> map = prices.stream().collect(toMap(p -> new ProductPriceMergeKey(p), p->p));

newPriceMap.forEach((k, p) -> oldPriceMap.merge(k, p, (newPrice, oldPrice) -> newPrice.merge(oldPrice));
}

Спасибо за пример, распилил на 2 сущности, данные по которым сравниваю - в key, по которым мержить в value.
Код: java
1.
2.
3.
4.
5.
6.
7.
8.
9.
10.
11.
12.
13.
public Map<ProductKey, ProductValue> mergePriceLists (Map<ProductKey, ProductValue> curPrices, Map<ProductKey, ProductValue> newPrices){

          Map<ProductKey, ProductValue> mergedPrices = new HashMap<>();

          for (Map.Entry<ProductKey, ProductValue> entry : curPrices.entrySet()) {
               if (newPrices.containsKey(entry.getKey())){
                   //mergedPrices
               } else {
                   //just add to outputmap
               }
          }
          return mergedPrices;
      }



Осталось улучшить метод, который в зависимости от дат пилит новые объекты.

Код: java
1.
2.
3.
4.
5.
6.
7.
8.
9.
10.
11.
12.
13.
14.
15.
16.
17.
18.
19.
20.
21.
22.
23.
24.
25.
26.
27.
28.
29.
30.
31.
32.
33.
34.
35.
 private void mergePrices(Price curPrice, Price newPrice) {

            if (curPrice.begin.compareTo(newPrice.begin) == 1 && curPrice.end.compareTo(newPrice.end) == 1) { //++
                if (curPrice.value == newPrice.value) {
                    mergedPrices.add(new Price(++cur_id, curPrice.product_code, curPrice.number, curPrice.depart, newPrice.begin, curPrice.end, curPrice.value));
                } else {
                    mergedPrices.add(new Price(++cur_id, curPrice.product_code, curPrice.number, curPrice.depart, newPrice.begin, newPrice.end, newPrice.value));
                    mergedPrices.add(new Price(++cur_id, curPrice.product_code, curPrice.number, curPrice.depart, newPrice.end, curPrice.end, curPrice.value));
                }
            }
            if (curPrice.begin.compareTo(newPrice.begin) == -1 && curPrice.end.compareTo(newPrice.end) == -1) { //++
                if (curPrice.value == newPrice.value) {
                    mergedPrices.add(new Price(++cur_id, curPrice.product_code, curPrice.number, curPrice.depart, curPrice.begin, newPrice.end, curPrice.value));
                } else {
                    mergedPrices.add(new Price(++cur_id, curPrice.product_code, curPrice.number, curPrice.depart, curPrice.begin, newPrice.begin, curPrice.value));
                    mergedPrices.add(new Price(++cur_id, curPrice.product_code, curPrice.number, curPrice.depart, newPrice.begin, newPrice.end, newPrice.value));
                }
            }
            if (curPrice.begin.compareTo(newPrice.begin) == -1 && curPrice.end.compareTo(newPrice.end) == 1) { //++
                if (curPrice.value == newPrice.value) {
                    mergedPrices.add(new Price(++cur_id, curPrice.product_code, curPrice.number, curPrice.depart, newPrice.begin, curPrice.end, curPrice.value));
                } else {
                    mergedPrices.add(new Price(++cur_id, curPrice.product_code, curPrice.number, curPrice.depart, curPrice.begin, newPrice.begin, curPrice.value));
                    mergedPrices.add(new Price(++cur_id, curPrice.product_code, curPrice.number, curPrice.depart, newPrice.begin, newPrice.end, newPrice.value));
                    mergedPrices.add(new Price(++cur_id, curPrice.product_code, curPrice.number, curPrice.depart, newPrice.end, curPrice.end, curPrice.value));
                }
            }
            if (curPrice.begin.compareTo(newPrice.begin) == 1 && curPrice.end.compareTo(newPrice.end) == -1) { //++
                if (curPrice.value == newPrice.value) {
                    mergedPrices.add(new Price(++cur_id, curPrice.product_code, curPrice.number, curPrice.depart, newPrice.begin, newPrice.end, curPrice.value));
                } else {
                    mergedPrices.add(new Price(++cur_id, curPrice.product_code, curPrice.number, curPrice.depart, newPrice.begin, newPrice.end, newPrice.value));
                }
            }
//etc. . .



Нужно сравнить между собой начальную, конечную даты и цену, в зависимости от результатов создавать новые объекты.
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
06.09.2017, 18:02
    #39516703
Blazkowicz
Blazkowicz
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Рефакторинг/Применение паттернов
С трудом пытаюсь подобрать слова, потому что кроме матов в голове только название книги Clean Code by Robert C Martin.
Вы правда не знаете что скопировать\вставить это худший способ переиспользовать код. И что править в таком коде баги мучительно больно, потому что их придется править в каждой копии, которую вы создали. А ещё более мучительно в такой код добавлять изменения...

По делу напишу позже, когда глаза перестанут кровоточить.
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
06.09.2017, 18:44
    #39516726
Blazkowicz
Blazkowicz
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Рефакторинг/Применение паттернов
Blazkowicz,

Ваши основные ошибки в методе mergePrices().
1. Метод оперирует исключительно полями класса Price. Поэтому он должен находится в классе Price. А не где-попало. Вы нарушаете инкапсуляцию.
2. Независимо от сценария, вам нужно вызывать метод .add() и конструктор new Prices(). Так с какого перепуга эти методы вызывать в каждом сценарии отдельно. А не один раз и для любого сценария?
3. Ваш код абсолютно не читаем. Какой нормальный человек сходу поймет это условие
curPrice.begin.compareTo(newPrice.begin) == -1 && curPrice.end.compareTo(newPrice.end) == 1
? Условия тоже нужно инкапсулировать в отдельные методы. Причем методы должы совпадать с названиями этих сценариев в предметной области. Например:
curPrice.intersectDatesOf(newPrice)
curPrice.olderThan(newPrice)
4. Я понимаю что лямды и стримы для новичка это не так просто. Но не использовать Java 8 Date/Time API для сравнения и манипуляций временем это, я считаю, большое упущение.
5. Вы спрашивали про паттерны. Так вот copy constructor это паттерн, которого тут так не хватает.
6. Инкремент идентификатора БД на уровне Java - очень сомнительное решение.

В итоге мы получаем. Что в зависимости от двух периодов, нам нужно получить список новых периодов и из них сформировать копии объектов.

Код: java
1.
2.
3.
4.
5.
6.
7.
8.
9.
10.
11.
12.
13.
14.
class Price {     
    public Price(Price original, java.time.Period newPeriod) {
        ...
    }

    List<Price> mergeNew(Price newer) {
           List<Price> result = new ArrayList<>();
           List<Period> newPeriods = mergePeriods(newer.period);
           for(Period newPeriod : newPeriods) {
                 result.add(new Price(this, newPeriod));
           }
           return result;
    }  
}
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
07.09.2017, 13:06
    #39517300
Blazkowicz
Blazkowicz
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Рефакторинг/Применение паттернов
https://ru.wikipedia.org/wiki/Don’t_repeat_yourself
...
Рейтинг: 0 / 0
| Ответить | Цитировать | Написать  
Форумы / Java [игнор отключен] [закрыт для гостей] / Рефакторинг/Применение паттернов / 12 сообщений из 12, страница 1 из 1
Найденые пользователи ...
Разблокировать пользователей ...
Читали тему (0):
Читали форум (0):
Пользователи онлайн (0):
Польз. соглашение  
Полит. конфиден.  
созд. / загр.: 294ms
Закрыть
start [/forum/topic.php?fid=59&mobile=1&tid=2122608]:
 0ms
get settings:
 7ms
get forum list:
 10ms
check forum access:
 3ms
check topic access:
 3ms
track hit:
 43ms
get topic data:
 7ms
get forum data:
 2ms
get page messages:
 29ms
get tp. blocked users:
 1ms
others: 189ms
total:  294ms
x
x
Закрыть


Просмотр
0 / 0
Close
Debug Console [Select Text]