0

I've been developing a small application for work, and I've come across something I can't figure out.

In the following code, I have an ArrayList of a Custom Class called 'Product' that contains data of type 'String'. I use the .contains method on this ArrayList to ensure it doesn't contain a certain String.

My IDE gives me the warning of 'Suspicious call to java.util.Collections.contains: Given object cannot contain instances of String (expected Product)'.

I completely understand the above message, because I'm comparing two different Types, so how can it ever evaluate correctly? I'm thinking it must be because the 'Product' class contains the data I want to compare, it is defaulting to using the toString method on the Product class (I override this in the Class) and comparing it with the String I want to compare it against.

It seems like JVM black magic to me.

private void createOrderListing(List<String[]> orderList)
{
    //For each line of the order list file
    for(String[] s : orderList)
    {
        if(s.length >= 28) //OrderLine should be of this length
        {
            if (!s[0].equalsIgnoreCase("ProductCode") && !s[0].isEmpty()) //Makes sure we're not including headers
            {
                //How does this bit work?
                if(!productListing.contains(s[0]))
                {
                    OrderLine order = new OrderLine();
                    //References product code of Product against Order Line, if match, then pack sizes and other basic fields ammended as appropriate
                    boolean productFound = false;
                    for (Product p : productListing) 
                    {
                        if (s[0].contentEquals(p.getProductCode())) 
                        {
                            order.initialAmendOrderLine(p.getProductCode(), p.getProductName(), p.getPackSize(), p.getProductType());
                            productFound = true;
                        }
                    }
                    if(productFound)
                    {
                        order.setOrderValues(s);
                        orderListing.add(order);
                    }
                }
                //System.out.println("\nOrder Product is: " + order.getProductName()+ "\nOrder Pack Size is: " + order.getInternalPackSize());
            }
        }
    }
}

UPDATE

The reason this works as pointed out in the comments is that the block is always true (the .contains method is always false, the ! inverses this, hence true). Sorry for the confusion and pointing out my carelessness.

aliwaii
  • 3
  • 3
  • 2
    The `contains()` method *could* work if the `equals()` method of `Product` compare equals with a `String` parameter. It shouldn't, as that technically violates the contract of `equals()`, but it can do so. – Andreas Jun 14 '17 at 15:17
  • 1
    Which type is productListing? I assume as you say is something like List – sirandy Jun 14 '17 at 15:19
  • 1
    productListing is of type List – aliwaii Jun 14 '17 at 15:21
  • There's no black magic here. If `productListing` is a `List` then `productListing.contains(aString)` will always return `false`. If it ever returns `true` then you have heap pollution, most likely because you're using [raw types](https://stackoverflow.com/q/2770321/2891664) and put a `String` in the list by mistake, or `Product.equals(Object)` has been overridden in a way that breaks its general contract. See also [`List.contains(Object)`](http://docs.oracle.com/javase/8/docs/api/java/util/List.html#contains-java.lang.Object-). – Radiodef Jun 14 '17 at 15:23
  • 1
    @aliwaii Add the information in your question – Turtle Jun 14 '17 at 15:24
  • What is your evidence that it "works"? More specifically, what evidence do you have that the `if(!productListing.contains(s[0])) {...}` block is ever skipped? – Klitos Kyriacou Jun 14 '17 at 15:24
  • @Radiodef Untrue. He could've `override` the `equals` method of his `Product` object to return true if it is compared with a String. – Turtle Jun 14 '17 at 15:25
  • @aliwaii That wasn't in your question, but I added a workaround in my answer, that you may want to look at: the [streams](https://docs.oracle.com/javase/8/docs/api/java/util/stream/package-summary.html). – Turtle Jun 14 '17 at 15:29
  • 1
    @KlitosKyriacou You are correct, the block is never skipped. Unfortunately the code in general worked perfectly when I added this section, and it wasn't before, meaning I assumed it had corrected the issue without reasoning on the why. – aliwaii Jun 14 '17 at 15:49
  • I suppose we should close the question as "Problem caused by a typographical error or not succeptible to help another user in the future". Otherwise, user ΦXocę 웃 Пepeúpa ツ posted this answer, tho his answer contained an error on the type of productListing – Turtle Jun 15 '17 at 07:26

3 Answers3

0

productListing is a list of Product objects. Yet you are asking the list if it contains a specific String object -- which shouldn't ever happen.

What you should do is check if your Product#getProductCode is equal to your specific String. This can be acheived by using streams:

if(!productListing.contains(s[0])) // replace this 

 // with this
if (!productListing.stream().filter(o -> o.getProductCode().equals(s[0])).findFirst().isPresent())

What does this code do? It checks all your Product elements to find one whose myStringData attribute is equal to the String you're comparing.

Turtle
  • 1,626
  • 16
  • 26
0

Here is an implementation of contains method in ArrayList that I have in OpenJDK:

public boolean contains(Object o) {
    return indexOf(o) >= 0;
}
public int indexOf(Object o) {
    if (o == null) {
        for (int i = 0; i < size; i++)
            if (elementData[i]==null)
                return i;
    } else {
        for (int i = 0; i < size; i++)
            if (o.equals(elementData[i]))
                return i;
    }
    return -1;
}

Basically, there is nothing complex in it. It iterates through the all elements of your ArrayList and checks whether your given object is equal to the current one. If the condition is true then element exists in the list.

So let's imagine that you are passing String "SomeValue" to this method. Elements of ArrayList are iterated and following action is executed: "SomeValue".equals(elementData[i]) where elementData[i] is a product.

Since equals method of String class cannot compare String with a Product it returns false and as a result, you get false from contains method.

To fix this situation you can iterate over ArrayList manually and compare some Product's field with your string. E.g. you can implement following contains method:

public boolean contains(List<Product> products, String yourStringValue) {
    for (Product p : products) {
        if(p.getProductCode().equals(yourStringValue)){
             return true;
        }
    }
    return false;    
}
nikita_pavlenko
  • 658
  • 3
  • 11
-1

since contains relays on equals implementation, when you do

 if(!productListing.contains(s[0]))

you are asking the list OF ARRAYS OF STRINGS if its contains a String.

that will return always false because the type are different, so is not that is working at all, is that your condition will always return false

ΦXocę 웃 Пepeúpa ツ
  • 47,427
  • 17
  • 69
  • 97