silent-box
silent-box

Reputation: 1666

How to avoid nested for-each loops?

I've got a class which should parse an XML file, like this:

<customers>
        ...
        <customer>
            <id>123456</id>
            <name>Mike</name>
            <orders>
                ...
                <order>
                    <id>233658</id>
                    <positions>
                        ...
                        <position>
                            <id>12345</id>
                            <price>10.0</price>
                            <count>5</count>
                        </position>
                        ...
                    </positions>
                </order>
                ...
            </orders>
        </customer>
<customers>

I'm going to unmarshall it with JAXB and than process result objects to get statistics (like max order amount, total orders amount etc)

Is this a bad practice to use 3-level foreach loop in this case?

public void getStatistics() {
    for (Customer customer: this.customers.getCustomer()) {

        BigDecimal customerTotalAmount = new BigDecimal(0);
        for (Order order : customer.getOrders().getOrder()) {

            BigDecimal orderAmount = new BigDecimal(0);
            for (Position position : order.getPositions().getPosition()) {
                orderAmount = orderAmount.add( position.getPrice().multiply(new BigDecimal(position.getCount())) );
            }

            customerTotalAmount = customerTotalAmount.add(orderAmount);
            this.totalOrders++;
        }

        this.totalAmount = this.totalAmount.add(customerTotalAmount);
    }
}

Customer, Order and Position classes has been generated automatically from XSD schema, and i think it is not good to change them.

What am i doing wrong? How can i avoid those nested loops?

Thank you.

Upvotes: 3

Views: 1813

Answers (1)

CptBartender
CptBartender

Reputation: 1220

I would recommend extracting some methods:

public void getStatistics() {
    for (Customer customer: this.customers.getCustomer()) {
        BigDecimal customerTotalAmount = processCustomer(customer);
        this.totalAmount = this.totalAmount.add(customerTotalAmount);
    }
}

private void processCustomer(Customer customer){
    BigDecimal customerTotalAmount = new BigDecimal(0);
    for (Order order : customer.getOrders().getOrder()) {
        BigDecimal orderAmount = new BigDecimal(0);
        for (Position position : order.getPositions().getPosition()) {
            orderAmount = orderAmount.add( position.getPrice().multiply(new BigDecimal(position.getCount())) );
        }

        customerTotalAmount = customerTotalAmount.add(orderAmount);
        this.totalOrders++;
    }
    return customerTotalAmount;
}

Do the same thing for the Order and Position loop, give the methods descriptive-enough names and make sure they return proper values, and you get a nice, clean code. These are still nested loops, but at least your eyes don't hurt when you see them.

Upvotes: 4

Related Questions