Posted By

Cur3n4 on 10/01/11


Tagged


Versions (?)

Answer question 4


 / Published in: Java
 

The debug parameter doesn't seem like a good idea and it is not used, so it should be removed.

list.size() will be evaluated in every loop iteration, so it should be assigned to a variable and the variable should be used in the for loop expression.

The first element is always retrieved from the order list.

ArrayList allow null elements but the null check should be done before using the order variable, otherwise a NullPointerException will be thrown. A null check should also be added for order.getProducts().

The found elements can be added directly within the inner loop statement, the found variable is not required. The check of product was using reference equality (==), it should be using equals(), the equals implementation of product should also be checked. Also the check won't work unless the product p is the last product in the list.

The whole inner loop should be replaced by a contains check.

There is no reason to define l as an ArrayList and then create a LinkedList, it should be defined as a LinkedList and be returned.

I would suggest having a look to Guava to improve the code below and also writing unit tests as the original code would not work as expected.

I would create a public boolean doesOrderContainProduct method in the Order class to simplify this method and improve encapsulation.

Also logging something could be useful.

Note: the code below if the list of products of an order contains null elements, and p is null, those orders will be returned. This is probably ok but it will be good to double check.

  1. public LinkedList findOrdersForProduct(Product p) {
  2. ArrayList list = getAllOrders();
  3. if (list != null) {
  4. int size = list.size();
  5. for (int i=0; i<size; i++) {
  6. Order order = (Order) list.get(i);
  7. if (order != null && order.getProducts() != null) {
  8. if (order.getProducts().contains(p))
  9. l.add(order);
  10. }
  11. }
  12. }
  13. return l;
  14. }
  15.  
  16.  
  17.  
  18.  
  19. }

Report this snippet  

You need to login to post a comment.