Comparing generic items?

Question

I am currently stuck on a particular part of my code. For my class, we are to create a train containing boxcars containing people or cargo. We use generics to define whether a boxcar can hold people or cargo. Then we load individual people/cargo onto the boxcar and if it has the same String "ID" as someone already on the boxcar then we log an error and do not load that person/cargo. This is where I'm having trouble. I cannot for the life of me figure out how to compare their "ID's" to see if they are equal. The following is the code that I have so far,

package proj5;

public class Person implements Comparable<Person> {

private String id;
private String name;
private int age;

public Person(String id, String name, int age){
    this.id = id;
    this.id = id;
    this.name = name;
    this.age = age;
}   

public Person(String id){
    this.id = id;
}

public String getId(){
    return id;
}

public int getAge(){
    return age;
}

public String getName(){
    return name;
}

public String toString(){
    String str = "        " + "ID: " + id + "  " + " Name: " + name + "  " + " Age: " + age;
    return str;
}

public int compareTo(Person p) {
    int result = this.id.compareTo(p.getId());
    return result;
}

}

package proj5;

import java.util.ArrayList;
import java.util.List;
import java.util.Collections;

public class Boxcar<T extends Comparable<T>> {

private ArrayList<T> boxcar;
private int maxItems;
private int boxcarID;

public Boxcar(){
    boxcar = new ArrayList<T>();
}

public void load(T thing){
    for(int i = 0; i < boxcar.size(); i++){
    if(boxcar.size() < maxItems && !boxcar.get(i).equals(thing)){
        boxcar.add(thing);
        System.out.println(boxcar.get(i));
    }
    else{
        boxcar.remove(thing);
    }
    }
    Collections.sort(boxcar);
}

public int getBoxcarId(){
    return boxcarID;
}

public int getMaxItems(){
    return maxItems;
}

public void setMaxItems(int i){
    maxItems = i;
}

public void unload(T thing){
    for(T item : boxcar){
        if(item.equals(thing)){
            boxcar.remove(item);
        }
    }
}

public List<T> getBoxcar(){
    return boxcar;
}

public String toString(){
    String str = "";
    for(T item : boxcar){
        str += item + "\n";
    }
    return str;
}

}

The problem is with my load function. I do not know how to compare their ID's. For clarification, the objects ID's are Strings. I have other classes but I have only included the ones I thought were necessary. If you need more files I will be happy to provide them. I have been stuck on this for hours and would appreciate any help! Thank you very much in advance!

EDIT: I have tried using the contains() method from the Collections API but why does that not work? It sounds like it would work perfectly.

Answer 1

You have to implement the equals and hashCode for your class Person.

The problem is that boxcar.get(i).equals(thing) is calling the generic equals and its only comparing the references.

so a generic equals will look like.

public boolean equals(Object obj){
  if (obj == null) return false;
  if (obj == this) return true;
  if (obj instanceof Person){
    Person p = (Person) obj;
    return p.getId().equals(this.getId());
  }
  return false;
}

and hashCode can be like this

public int hashCode(){
  return 37*this.getId().hashCode();
}

Answer 2

I think there the problem is because of boxcar = new ArrayList<T>(); because it can contain people/cargo. if its contains all people object then your code is ok. But because it can also contain cargo so that it showing problem.

So what you can do is just check if its object of person then only check for compare.

public boolean equals(Object obj){
 if (obj instanceof Person){
   Person p = (Person) obj;
   return p.getId().equals(this.getId());
 }
 return true;
}

And if you override equals method then u also have to override hashCode method to

Answer 3

The accepted answer is ok for your use and actually will work for testing equality, but thinking about it for a second it does leave open a problem: Because your Boxcar class is a Generic, this will rely on whatever it's holding to have implemented equals() and hashcode(). You also have a problem with your load() method itself.

Since you restrict the generic with <T extends Comparable<T>> then you can simply do:

if (boxcar.get(i).compareTo(thing) == 0) {

to check for equality since compareTo() is going to return 0 on an exact match.

Your load() method is incorrect though; it's iterating through the ArrayList and adding the same thing over and over. To meet your criteria as you posted it needs to be something like:

public void load(T thing) {
    boolean found = false;

    // Go through ArrayList and see if thing exists
    for (int i = 0; i < boxcar.size(); i++){
        if (boxcar.get(i).compareTo(thing) == 0) { 
            System.out.println(thing + " already exists");
            found = true;
            break;
        }
    }

    // If the thing didn't exist, add it, and sort the ArrayList    
    if (!found) {
        boxcar.add(thing);
        System.out.println("Added " + thing);
        Collections.sort(boxcar);
    }
}

Now it'll work for both your People and your Cargo

And now ... for bonus points, since you're sorting your ArrayList every time you add an item, you can use a binary search rather than a linear one. Conveniently, one is provided in the Java Collections class for exactly this - it relies on a sorted list holding things that implement Comparable:

public void load(T thing) {

    // See if the thing exists. binarySearch returns an index if it's there
    // or a negative number if it's not
    if (Collections.binarySearch(boxcar, thing) >= 0) {
        System.out.println(thing + " already exists");
    } else {
        boxcar.add(thing);
        System.out.println("Added " + thing);
        Collections.sort(boxcar);
    }
}

Edit to add due to comments: You would also need to change your unload() method. Again, we're using compareTo() to check equality. Also ... you're not breaking out of your loop once you find it. Since we know there can't be duplicates from the way load works and you never want to modify a list while iterating through it:

public void unload(T thing){
    for(T item : boxcar){
        if(item.compareTo(thing) == 0) {
            boxcar.remove(item);
            break;
        }
    }
}

Note there is a slight nuance here. We make sure to use item with remove(). This is because remove() relies on equals() which by default will use the reference value of the object. item and thing compare equally, but are two different instances of the same class. By giving remove() the item it knows to remove that instance from the list.

Better yet since it's faster and avoids this ... use binarySearch() and remove via the index!

public void unload(T thing){
    int index = Collections.binarySearch(boxcar, thing);
    if (index >=0) {
        boxcar.remove(index);
    }
}