1

I am trying to compare two objects from the same class that contains both String and Int parameters.

This is for my lab assignment, and I have tried to use just .equals method without overriding it, did not work, I did some more research on how to compare the String and Int parameters of both objects. So I build a override equals and it only worked with String parameters as I tested each parameter, the Int parameter keeps returning as True, so i am not sure where i am going wrong.

Updated Ok, now i got it working, I'm trying to figure out to return false if Names are different, or if Team names are different. I thought of using if and else but nested if-else?

//This is my equals method to compare the String and Int parameter.
public boolean equals(Object o)
   {
       Player comp = (Player) o;
       if (o == this) {
           if (comp.team.equals(this.team)) {
               return true;
           } else if (comp.jerseyNumber == this.jerseyNumber) {
               return true;
           }

       }else
           return false;
   }
//And this is the method that calls on Player.
      player1.readPlayer();
//Prompt for and read in information for player 2
       player2.readPlayer();
//Compare player1 to player 2 and print a message saying
//whether they are equal
       if (player1.equals(player2))
       {
           System.out.println("Same Players");
       }else
       {
           System.out.println("Different Players");
       }

So i expected the result to print out as either Same Players if Name, Team and jersey number are the same, or Different Players if all three are different, I think. Instead with all three parameters it prints out as Same player regardless the Int parameter value, when I grayed out the Int Parameter, it actually showed both Same and Different Players result.

  • Ok, now i got it working, I'm trying to figure out to return false if Names are different, or if Team names are different. I thought of using if and else but nested if-else? – Stargazer7861 Aug 04 '19 at 01:54
  • You say `if (o == this)`--think about the meaning of that for a moment. – chrylis -cautiouslyoptimistic- Aug 04 '19 at 02:14
  • well I would be calling on Object o to compare to this which is the class's parameter, right? I'm bad with terminology – Stargazer7861 Aug 04 '19 at 02:18
  • You are literally saying `if (other object is the same object as this object)`, in which case the result is instatrue. (By the way, you also need to check what happens when you try `player.equals("not a player")`.) – chrylis -cautiouslyoptimistic- Aug 04 '19 at 02:20
  • I fixed it, it looks like so ```java public boolean equals(Object o) { if (o == this) return true; Player comp = (Player) o; if (!comp.team.equals(this.team)) return false; if (comp.jerseyNumber != this.jerseyNumber) return false; else return true; } ``` – Stargazer7861 Aug 04 '19 at 02:22
  • @Stargazer7861 I suggest looking at auto-generated `equals` methods from Eclipse - they do null, reference, and type checking before doing actual field comparisons. To @chrylis point, it's certainly fair to check `o == this` and return true, but there are also other safety checks you should have. – josh.trow Aug 04 '19 at 03:30
  • @josh.trow If they do null checking, then that's bad auto-generated code; `o instanceof Player` will evaluate to false if `o` is null. – chrylis -cautiouslyoptimistic- Aug 04 '19 at 03:32
  • interesting so o instanceof Player will ensure that null checking will go through without causing any issues? – Stargazer7861 Aug 04 '19 at 16:36
  • @chrylis I'm just going to link to the best discussion I can find on why it does what it does and note that I prefer the absolute knowledge that my items are equal and symmetrical (by using getClass): https://stackoverflow.com/questions/596462/any-reason-to-prefer-getclass-over-instanceof-when-generating-equals – josh.trow Aug 06 '19 at 00:13
  • @josh.trow `getClass` is an entirely reasonable approach that does in fact justify null check. – chrylis -cautiouslyoptimistic- Aug 06 '19 at 17:01

1 Answers1

0

You have a self-comparison in your equality check:

Player comp = (Player) o;
if (comp.team.equals(this.team))
    return true;
if (comp.jerseyNumber == ((Player) o).jerseyNumber) <== RIGHT HERE
    return true;

I assume you intended:

if (comp.jerseyNumber == this.jerseyNumber)

Based on the comp variable, I'm guessing you extracted the cast but missed one, then got a bit tied up on review.

EDIT: I would also suggest using quick-exits, like so - otherwise, if the teams are the same, but jerseys different, the way you have it today will say they are the same person:

Player comp = (Player) o;
if (!comp.team.equals(this.team))
    return false;
if (comp.jerseyNumber != this.jerseyNumber)
    return false;
return true;
josh.trow
  • 4,861
  • 20
  • 31
  • oh, funny thing I thought of trying ```java jerseyNumber == this.jerseyNumber ``` and it works, but I think that is better and ensuring that it will check comp.jerseyNumber. – Stargazer7861 Aug 04 '19 at 01:46
  • Thanks! :D that worked for me, I still struggle with boolean logic and applying it toward coding, I have partly myself to blame for not practicing much, but my motivation for coding is bit meh and I really want to improve my skills on it – Stargazer7861 Aug 04 '19 at 02:07