1
\$\begingroup\$

I have a list of orders and for some order fields I need get the data that is common among the orders. If the data is not in common, I should indicate null. I collect the common data in a CommonData object. In addition, I also store the order codes and order ids associated with the CommonData.

I have two approaches that I'm considering:

Approach 1:

public CommonData getCommonData(List<Order> orders) {       
    Order firstOrder = order.get(0);

    LocalDate commonStartDate = firstOrder.getStartDate();
    LocalDate commonEndDate = firstOrder.getEndDate();
    List<Item> commonItems = firstOrder.getItems();

    CommonData commonData= new CommonData();

    for (Order order : orders) {

        commonData.addCode(order.getCode());
        commonData.addId(order.getId());

        if (commonStartDate != null &&
                !commonStartDate.equals(order.getStartDate())) {
            commonStartDate = null;
        }

        if (commonEndDate != null &&
                !commonEndDate.equals(order.getEndDate())) {
            commonEndDate = null;
        }

        if (!equalLists(commonItems, order.getItems())) {
            commonData.setDifferentItems(true);
            commonItems = null;
        }
    }

    commonData.setCommonStartDate(commonStartDate);
    commonData.setCommonEndDate(commonEndDate);
    commonData.setCommonItems(commonItems);
    return commonData;
}

Approach 2:

private boolean haveSameDate(List<Order> orders, 
        Function<Order,LocalDate> getDate) {

    return orders.stream()
            .map(getDate)
            .distinct()
            .limit(2)
            .count() == 1;
}

public CommonData getCommonData(List<Order> orders) {       
    CommonData commonData = new CommonData();

    if (haveSameDate(orders, 
            Order::getStartDate)) {
        commonData.setCommonStartDate(orders.get(0).getStartDate());
    }

    if (haveSameDate(orders, 
            Order::getEndDate)) {
        commonData.setCommonEndDate(orders.get(0).getEndDate());
    }

    commonData.setCodes(
            orders.stream()
                .map(order -> order.getCode())
                .collect(Collectors.toList()));

    commonData.setIds(
            orders.stream()
                .map(order -> order.getId())
                .collect(Collectors.toList()));

    List<Item> firstOrderItems = orders.get(0).getItems(); 
    if (orders.stream()
        .map(Order::getItems)
        .allMatch(x -> equalLists(x,firstOrderItems))) {            

        commonData.setCommonItems(firstOrderItems);
    } else {
        commonData.setDifferentItems(true);
    }

    return commonData;
}

Even though approach 2 involves multiple iterations over orders, the order and item lists will always be small.

In approach 2, having operations on one piece of data is in one spot within the method but it is scattered in approach 1. Approach 1 also has negative logic. Approach 2 is more complex in finding common order items.

Can approach 1 be redesigned to have the benefits of approach 2 (such as remove negative logic and keep operations on a piece of data in a single spot)? Or should approach 2 be used and perhaps be improved to better handle finding common order items?

\$\endgroup\$
2
\$\begingroup\$

Basically, this boils down to the question of how to find out whether a given number of items is always the same. The mathematical tool for this is a set and checking its cardinality.

Putting this into code, you can use a Set implementation in Java:

Set<LocalDate> allStartDates = new HashSet<>();

for (...) {
    ...
    allStartDates.add(order.getStartDate());
    ...
}

if (allStartDates.size() == 1)
    commonData.setCommonStartDate(allStartDates.iterator().next());

BUT: in a real-world-application, I'd move this logic to the CommonData class.

Basically, redesign the CommonData in a way, that you can simply add an order and then boil the loop down to:

CommonData commonData = new CommonData();
orders.stream().forEach(commonData::add);

and inside the common data add method, add the codes and ids, maintain two sets of dates, and do the if-logic in the appropriate getters.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.