Информация об изменениях

Сообщение Re[5]: Уменьшить Cognitive Complexity метода. рефакторинг. от 14.09.2018 18:44

Изменено 14.09.2018 18:48 Artem Korneev

Re[5]: Уменьшить Cognitive Complexity метода. рефакторинг.
Здравствуйте, Слава, Вы писали:

С>Я бы не сказал, что все эти манипуляции с Map<String, Function<IndexedDraftOrderDto, String>> прямо-таки уменьшили Cognitive Complexity. Скорее уж — увеличили. Был код простой, стал затейливый.


Это все-таки субъективно. Мне декларативный код читать проще и быстрее. И тот и другой вариант сравнительно просты. Но исходный код линейно усложняется с увеличением количества колонок.

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

    private void fillCvsColumnsValues(ExportOrderRequestDto request, ZoneId zoneId, DateTimeFormatter dateFormat,
                                      CsvBuilder csvBuilder, PageDto<IndexedDraftOrderDto> searchOrders) {
        for (IndexedDraftOrderDto o : searchOrders.getContent()) {

            if (request.getColumns().contains(ExportColumnEnum.ORDER_CODE)) {
                csvBuilder.addNextColumnValue(o.getOrderCode());
            }

            if (request.getColumns().contains(ExportColumnEnum.MARKET)) {
                csvBuilder.addNextColumnValue(o.getNumberOfAnimals());
            }

            if (request.getColumns().contains(ExportColumnEnum.STATUS)) {
                csvBuilder.addNextColumnValue(isNull(o.getOrderState()) ? "" : ORDER_STATE_DESC.get(o.getOrderState()));
            }

            if (request.getColumns().contains(ExportColumnEnum.CUSTOMER)) {
                String name = nonNull(o.getCustomer()) ? o.getCustomer().getName() : "";
                csvBuilder.addNextColumnValue(name);
            }

            if (request.getColumns().contains(ExportColumnEnum.ANIMAL_COUNT)) {
                csvBuilder.addNextColumnValue(o.getNumberOfAnimals());
            }

            if (request.getColumns().contains(ExportColumnEnum.SAVED)) {
                csvBuilder.addNextColumnValue(formatDate(o.getSavedDate(), zoneId, dateFormat));
            }

            if (request.getColumns().contains(ExportColumnEnum.PRODUCT)) {
                List<String> products = Optional.ofNullable(o.getProducts()).orElse(Collections.emptyList()).stream()
                                                .map(IndexedProductDto::getProductName)
                                                .collect(Collectors.toList());
                csvBuilder.addNextColumnValue(products);
            }

            if (request.getColumns().contains(ExportColumnEnum.MARKET)) {
                csvBuilder.addNextColumnValue(o.getMarket());
            }

            csvBuilder.endRow();
        }


В варианте с лямбдами Map просто не даст добавить один элемент дважды в инициализаторе. Баг с правильностью преобразования заметить тоже будет непросто, но все-таки больше надежды, что глаз заметит неточность, когда все правила преобразования выведены в простую табличку, отдельно от кода перебора и добавления колонок:

    put(ExportColumnEnum.ANIMAL_COUNT, (o) -> o.getNumberOfAnimals());
    put(ExportColumnEnum.SAVED,        (o) -> formatDate(o.getSavedDate(), zoneId, dateFormat));    
    put(ExportColumnEnum.PRODUCT,      (o) -> getProduct(o));
    put(ExportColumnEnum.MARKET,       (o) -> o.getNumberOfAnimals());


Я очень часто вижу ошибки, сделанные при копи-пасте. Декларативные подходы позволяют значительно уменьшить копи-паст и вероятность таких ошибок.
Re[5]: Уменьшить Cognitive Complexity метода. рефакторинг.
Здравствуйте, Слава, Вы писали:

С>Я бы не сказал, что все эти манипуляции с Map<String, Function<IndexedDraftOrderDto, String>> прямо-таки уменьшили Cognitive Complexity. Скорее уж — увеличили. Был код простой, стал затейливый.


Это все-таки субъективно. Мне декларативный код читать проще и быстрее. И тот и другой вариант сравнительно просты. Но исходный код линейно усложняется с увеличением количества колонок.

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

    private void fillCvsColumnsValues(ExportOrderRequestDto request, ZoneId zoneId, DateTimeFormatter dateFormat,
                                      CsvBuilder csvBuilder, PageDto<IndexedDraftOrderDto> searchOrders) {
        for (IndexedDraftOrderDto o : searchOrders.getContent()) {

            if (request.getColumns().contains(ExportColumnEnum.ORDER_CODE)) {
                csvBuilder.addNextColumnValue(o.getOrderCode());
            }

            if (request.getColumns().contains(ExportColumnEnum.MARKET)) {
                csvBuilder.addNextColumnValue(o.getNumberOfAnimals());
            }

            if (request.getColumns().contains(ExportColumnEnum.STATUS)) {
                csvBuilder.addNextColumnValue(isNull(o.getOrderState()) ? "" : ORDER_STATE_DESC.get(o.getOrderState()));
            }

            if (request.getColumns().contains(ExportColumnEnum.CUSTOMER)) {
                String name = nonNull(o.getCustomer()) ? o.getCustomer().getName() : "";
                csvBuilder.addNextColumnValue(name);
            }

            if (request.getColumns().contains(ExportColumnEnum.ANIMAL_COUNT)) {
                csvBuilder.addNextColumnValue(o.getNumberOfAnimals());
            }

            if (request.getColumns().contains(ExportColumnEnum.SAVED)) {
                csvBuilder.addNextColumnValue(formatDate(o.getSavedDate(), zoneId, dateFormat));
            }

            if (request.getColumns().contains(ExportColumnEnum.PRODUCT)) {
                List<String> products = Optional.ofNullable(o.getProducts()).orElse(Collections.emptyList()).stream()
                                                .map(IndexedProductDto::getProductName)
                                                .collect(Collectors.toList());
                csvBuilder.addNextColumnValue(products);
            }

            if (request.getColumns().contains(ExportColumnEnum.MARKET)) {
                csvBuilder.addNextColumnValue(o.getMarket());
            }

            csvBuilder.endRow();
        }


В варианте с лямбдами Map просто не даст добавить один элемент дважды в инициализаторе. Баг с правильностью преобразования заметить тоже будет непросто. Но все-таки есть больше надежды, что глаз заметит неточность, когда все правила преобразования выведены в простую табличку, отдельно от кода перебора и добавления колонок:

    put(ExportColumnEnum.ANIMAL_COUNT, (o) -> o.getNumberOfAnimals());
    put(ExportColumnEnum.SAVED,        (o) -> formatDate(o.getSavedDate(), zoneId, dateFormat));    
    put(ExportColumnEnum.PRODUCT,      (o) -> getProduct(o));
    put(ExportColumnEnum.MARKET,       (o) -> o.getNumberOfAnimals());


Я очень часто вижу ошибки, сделанные при копи-пасте. Декларативные подходы позволяют значительно уменьшить копи-паст и вероятность таких ошибок.