What am I missing?

Question about the task

Under discussion

When I run the program, it says true. if I change one of the parameters for one of the Iphones, I get false, yet I am not meeting the second criteria?

The main creates two iPhones with identical properties.
They are compared and the result is displayed on the console. Figure out why the result is currently negative and make it so the result is positive. To do this, you need to override the equals(IPhone) method and account for all the phone properties.
The values of fields must be the same if two objects are equal. The main method is not checked.

Requirements:

The IPhone class must override the public boolean equals(Object) method.
The equals method must return true for two equal IPhone objects and false for different ones.
The equals method must return false if null is passed to it.

package en.codegym.task.pro.task10.task1010; import java.util.Objects; /* Two iPhones */ public class Iphone { private String model; private String color; private int price; public Iphone(String model, String color, int price) { this.model = model; this.color = color; this.price = price; } @Override public boolean equals(Object ob) { if(this == ob) return true; if(!(ob instanceof Iphone)) return false; Iphone iphone = (Iphone) ob; if(this.equals(null) || iphone.color.equals(null) || iphone.model.equals(null)) return false; return this.model.equals(iphone.model) && this.color.equals(iphone.color) && this.price == iphone.price; } public static void main(String[] args) { Iphone iphone1 = new Iphone("X", "Black", 999); Iphone iphone2 = new Iphone("X", "Black", 999); System.out.println(iphone1.equals(iphone2)); } }

Comments (5)

Popular
New
Old

You must be signed in to leave a comment

Bob

Level 10 , United States of America, United States

14 July, 20:43

Thank you Thomas that helped! I always get mixed up with the nested if statements. I have no experiance with ternary operators yet so I didn't want to just copy code.

Bob

Level 10 , United States of America, United States

14 July, 15:30

Thank you. i will continue fighting the null part. I got rid of that equals(null) code and used == to verify model and color if they equal null. Now I get a nullPointerException if one is null and one is not. So I will chase that down.

Thomas

Level 41 , Bayreuth, Germany

14 July, 16:58

useful

Don't they explain that in detail anymore? If you are supposed to fight your way through then it is kinda difficult. Usually it is a scheme you work through, like:

public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || this.getClass() != o.getClass()) return false;
        // if (!(o instanceof Iphone)) return false;

        Iphone that = (Iphone) o;

You either use getClass or instanceof. The latter does not need the explicit null check of o but on the other hand it checks descendants as well and considers them as equal (eg a fruit could be equal to an apple) Then you do the ckecking. The easy part is the primitives:

if (this.price != that.price) return false;

If primitives are not equal you do not need to further ckeck and can leave the equals method. The same we do now using the color field. But here we need null comparision in addition.

if (this.color == null) {
    if (that.color != null) return false; // if this.color == null && that.color != null -> return false
} else if (!this.color.equals(that.color)) return false; // if this.color != null && !this.color.equals(that.color) -> return false
// if this.color.equals(that.color) continue with the next check

I hope you can understand my comments... Because this is a little bit lengthy one usually writes that using ternary operators:

if (this.color == null ? that.color != null : !this.color.equals(that.color)) return false;

Today you use the Objects utility class

if (!Objects.equals(this.color, that.color)) return false;

Thomas

Level 41 , Bayreuth, Germany

14 July, 17:02

For the last check you do not want to return just false if the model objects are not equal but true or false depending on the result of the comparison. That's why the check works different:

return this.model == null ? that.model == null : this.model.equals(that.model);

Or using the Objects class:

return Objects.equals(this.model, that.model);

I haven't really tested my code but it should work (fingers crossed ;) ) And you always can ask your IDE to write the code for you: ALT+insert -> equals() and hashcode()

Thomas

Level 41 , Bayreuth, Germany

14 July, 14:56

Imho the way you handle null fields is not working as it should. - Starting with

this.equals(null)

this never can be null. You can not check if this == null using this. It either is null or you can use it, not both. - Fields can be null. That is not an error and you can create valid objects that way, eg.:

new Iphone("X", null, 999);

Your code however will throw a NullPointerException if I do. - Last point will be still related to null fields. Of course objects having null fields should be considered equal if null fields match as well as the other fields, eg.:

Iphone iphone1 = new Iphone("X", null, 999);
Iphone iphone2 = new Iphone("X", null, 999);
System.out.println(iphone1.equals(iphone2)); // expected: true