Implement CompareTo Correctly

Question

I've got the following class:

//GetHasCode, toString, and equalsTo removed to keep the question simple.
private String weaponName;
private String weaponType;
private int weaponDamage;

public WeaponObject(String name, String type, int damage)
{
    this.weaponName = name;
    this.weaponType = type;
    this.weaponDamage = damage;
}

@Override
public int compareTo(WeaponObject compare) {

int name = this.getWeaponName().compareTo(compare.getWeaponName());
int type = this.getWeaponType().compareTo(compare.getWeaponType());
int damage = Integer.compare(this.weaponDamage, compare.getWeaponDamage());

  if(name !=0 )
  {
    return name;
  }
  if(type != 0)
  {
      return type;
  }
  if(damage != 0)
  {
     return damage;
  }
  return 0;

}

SubClass:

public class Sword extends WeaponObject {


    private String swordAttahment;

    public Sword(String name, String type, int damage, String attachment) {
        super(name, type, damage);
        this.swordAttahment = attachment;
    }


    public String getSwordAttahment() {
        return swordAttahment;
    }


    @Override
    public int compareTo (WeaponObject compare)
    {
        int superCompare = super.compareTo(compare);

        if(superCompare != 0)
        {
            return superCompare;
        }

        Sword other = (Sword)compare;

        int attach = this.getSwordAttahment().compareTo(other.getSwordAttahment());

        if(attach != 0)
        {
            return attach;
        }

        return 0;
    }

Questions:

1. Given that I have a Sword class that extends WeaponObject, have I correctly implemented my compareTo in the Sword class?

2. If the above isn't correct, then how would I correctly implement the compareTo method in my subclass?

Yetac

Sword.compareTo() will throw a ClassCastException if you try comparing to a WeaponObject. – Butacaine

@Butacaine - How do I fix it? – Yetac

WeaponObject doesn't have getSwordAttahment() method. So you can not make a comparison based on swordAttahment. – Curtice

@Svetlana checking instanceof is the obvious fix, but there are deeper problems here that are much harder to solve, as per my comment on @rdonuk's answer. My suggestion would be to think hard about whether Weapon and Sword both need to implement Comparable and override compareTo(), or if there's another setup that'll work for you. On a side note, WeaponObject is no more descriptive a class name than Weapon. – Butacaine

See the discussion here for more info. – Butacaine

Answer 1

WeaponObject doesn't have getSwordAttahment() method. So you can not make a comparison based on swordAttahment. You can use instanceof to avoid ClassCastException

@Override
public int compareTo (WeaponObject compare)
{
    int superCompare = super.compareTo(compare);

    if(superCompare != 0)
    {
        return superCompare;
    }

    if(compare instanceof Sword) {
        Sword other = (Sword)compare;

        int attach = this.getSwordAttahment().compareTo(other.getSwordAttahment());

        if(attach != 0)
        {
            return attach;
        }
    }

    return 0;
}

Curtice

One more question, could I do this.getClass != compare.getClass() return -1? Instead of instanceof? – Yetac

It is your desicion. If you are thinking that "if the class types are different they must be different" you can write that. – Curtice

Many thanks for your answer, and code. Much appreciated. – Yetac

@Svetlana, definitely not. You would be breaking symmetry, because w.compareTo(s) == 0 while s.compareTo(w) == -1. Returning 0 would be a better option, but even that could be breaking transitivity, because you can have a scenario where w.compareTo(s1) == 0 && w.compareTo(s2) == 0 && s1.compareTo(s2) != 0. You'll also have trouble if you extend Sword without overriding compareTo(). Either way you'll want to make sure equals() implements the same logic, as recommended by the documentation. – Butacaine

@Butacaine - Many thanks for your advice. For the small text adventure game I'm building I don't think compareTo will be implemented. – Yetac