0

I have a list of Custom object and i want to find an object by given an Id(a field in custom object). i was coding for this so i found two solutions when comparing fields.

1

private Product getProduct(String productId,List<Product> productList){
        for (int i = 0; i < productList.size(); i++) {
            if (productId.equals(productList.get(i).getId())) {
                return productList.get(i);
            }


        }
        return null;
    }

2.

 private Product getProduct(String productId,List<Product> productList){
        for (int i = 0; i < productList.size(); i++) {
            if (productList.get(i).getId().equals(productId)) {
                return productList.get(i);
            }
        }
        return null;
    }

The difference is in if condition , i want to know which one is better than the other and why, when to use 1st method and when to use second ?

Adnan Ali
  • 792
  • 1
  • 8
  • 21

4 Answers4

2

Since equals() is required by Java to be symmetric, there is no difference between the two snippets.

Both snippets are sub-optimal, in that they iterate by numeric index, and retrieve productList.get(i) twice before returning it. Iterating by index is especially dangerous, because passing a LinkedList<Product> will slow down your search considerably.

A better approach is to use a for-each form of the loop:

for (Product p : productList) {
    if (p.getId().equals(productId)) {
        return p;
    }
}
return null;
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
2

The concern in both of your implementations is the possibility of calling .equals on a null value.

If you can guarantee neither of them are null then they are equivalent.

If you are using Java 8, stream may be a better choice.

private Product getProduct(String productId,List<Product> productList){
    return products.stream()
        .filter(p-> productId.equals(p.getId())
        .findFirst()
        .orElse(null);
JynXXedRabbitFoot
  • 996
  • 1
  • 7
  • 17
0

The first one invokes equals on the parameter productId, while the second one invokes equals on the current list element from productList. The result is the same because equals is symmetric:

for any non-null reference values x and y, x.equals(y) should return true if and only if y.equals(x) returns true.

You can also use a stream for this, so you don't have to care about implementation details (furthermore, Objects#equals(Object, Object) is null-safe):

String p = productList.stream().filter(e -> Objects.equals(e, productId))
                               .findFirst()
                               .orElse(null);

Have a look a this question for further information.

Community
  • 1
  • 1
beatngu13
  • 7,201
  • 6
  • 37
  • 66
0

When you are sure the product id's are never null it doesn't really matter.

But in general it's always good to program in a defensive way, so for example prefer using

"SomeString".equals(aString)

instead of

aString.equals("SomeString")

since you know "SomeString" is never null.

Or use

Objects.equals(object1, object2)

when both objects might be null.

Wilco Greven
  • 2,085
  • 1
  • 10
  • 9