Reputation: 1666
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
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