List of objects vs string value comparison

Question

Hi , I expecting the following to return true.

   public class HudsonJob {

     private String name;
     private String status;
     public String getName() {
      return name;
     }
     public void setName(String name) {
      this.name = name;
     }
     public String getStatus() {
      return status;
     }
     public void setStatus(String status) {
      this.status = status;
     }

     public boolean equals(Object jobName) {
      return name.toLowerCase().equals(((String)jobName).toLowerCase());
     }

     public int hashCode() {
      return name.hashCode();
     }
    }

,

List<HudsonJob> existingJbsLst = hudsonUtil.getAllJobs(); // returns multiple HudsonJob objects in the list.

The statement I'm expecting to return true is :

boolean isExistingJob = existingJbsLst.contains("AnExistingJOB"); is always returning false.

OR boolean isExistingJob = existingJbsLst.equals("AnExistingJOB"); is also returning false.

What should I add/change in the code to get the expected return value.

Answer 1

• You should pass a HudsonJob object (not a String) to the contains(..) method. For example (if you add a constructor taking the name as parameter):

  boolean exists = existingJbsLst.contains(new HudsonJob("AnExistingJOB"));
  

• Let your IDE generate the equals and hashCode methods - it will add proper null checks, type checks, etc.

• You are breaking the contract of equals. See here
• use string.equalsIgnoreCase(..)

Answer 2

The second expression really should return false, because you compare a List to a String. If the first expression returns false, then obviously there's no such job in the list. The implementation of equals is correct (OK, you should test for 'null' and for same classes)

While writing about the missing test - equals is not implemented correctly, it should not take a job name but a HudsonJob object:

public boolean equals(Object obj) {
  if (obj == null) return false;
  if (!(obj instanceof HudsonJob)) return false;
  HudsonJob that = (HudsonJob) obj;
  return this.name.equals(that.name);
}

Answer 3

You brake the contract for equals method. For example:
It 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.
And in your case job.equals(string) may be true, but string.equals(job) will always be false.

And what probably happens in your list is that elements are compared another way:

for (Object el : list) {
    if (parameter.equals(el)) {
        ...
    }
}

That's the technical reason why it's not working: "AnExistingJOB".equals(jobObject) is always false.

edit
BTW, your hashCode method is wrong also. If comparing in equals method you ignore case, ignore it in hashCode too. Bozho's advice is probably a good one: IDE will generate those methods better. Also, you can check Andreas_D's answer for a valid implementation.

Answer 4

The "contains" operator loops through all the memers of the set, and compares the "want" value to the "found" value. I say that very carefully. If you say "joblist.contains(wantjob)", this gives -- to leave out some complexity, the relevant part:

for (HudsonJob gotjob : joblist)
{
  if (wantjob.equals(gotjob))
    return true;
}

That is, the comparison is "objectIAmLookingFor.equals(objectInList)", NOT "objectInList.equals(objectIAmLookingFor)".

So when you search a list of HudsonJob's for a String, it is using the String.equals function, NOT your HudsonJob.equals function. And String.equals knows nothing about HudsonJobs, so it promptly returns false.

There are two ways to do what you want.

One way is to change your HudsonJob.equals function to

public boolean equals(Object o)
{
  if (o==null || !(o instanceof HudsonJob))
    return false;
  return this.name.toLowerCase().equals(((HudsonJob)o).name.toLowerCase());
}

Then write

HudsonJob wantjob=new HudsonJob();
wantjob.setName("AnExistingJob");
if (existingJobList.contains(wantjob))
  ... whatever ...

The other way is to not use "contains" but instead write your own search function, like:

public boolean jobInList(List<HudsonJob> existingJobs, String wantJob)
{
  for (HudsonJob gotjob : existingJobs)
  {
    if (gotjob.name.toLowerCase().equals(wantJob.toLowerCase())
      return true;
  }
  return false;
}

(Actually if you need the toLowerCase it would be better to do the wantJob.toLowerCase before the loop, but whatever.)

Then you can say

if (jobInList(existingJobs,"AnExistingJob"))
... do something ...