Версия для печати темы
Нажмите сюда для просмотра этой темы в оригинальном формате
Форум программистов > Java: Общие вопросы > Некорректная имплементация equals()


Автор: Royan 17.10.2007, 22:52
Есть класс
Код

public class Test {
    private String name;
    public Test(String name) {
        this.name = name;
    }
    public boolean equals(Test t) {
        return t.name.equals(this.name);
    }
}

У кого-нибудь есть идеи почему это некорректная имплементация equals? Подчеркну, что речь идет именно о реализации метода equals(), а не о том, что тут не переписан hashCode()

Автор: Platon 17.10.2007, 22:57
Код

public class Test {
    private String name;
    public Test(String name) {
        this.name = name;
    }


    public boolean equals(Object obj) {
        Test t = (Test)obj;
        return t.name.equals(this.name);
    }
}


Добавлено через 44 секунды
Ты, наверно, имел ввиду переопределение.

Автор: Royan 18.10.2007, 00:00
Верно сообразил. Молодец Platon +1 за внимательность.

Автор: Platon 18.10.2007, 00:02
Че-то седня ты целый день вопросами сыпешь ;) я так с тобой авторитетным челом стану

Автор: fixxer 18.10.2007, 09:11
Цитата(Platon @ 17.10.2007,  22:57)
Код

public class Test {
    private String name;
    public Test(String name) {
        this.name = name;
    }


    public boolean equals(Object obj) {
        Test t = (Test)obj;
        return t.name.equals(this.name);
    }
}


ClassCastException не боитесь поймать?
Код

public boolean equals(Object obj) {
        if (this == obj) return true;
        if (!(obj instanceof Test)) return false;
        Test t = (Test)obj;
        return t.name.equals(this.name);
}

Автор: Platon 18.10.2007, 09:18
И правда что!! Приношу свои извинения, абсолютно правильная поправка.

Автор: Royan 18.10.2007, 09:35
fixxer, Да абсолютно верно это тоже надо учитывать.

Автор: mindflyer 18.10.2007, 10:10
ClassCastException  уже боимся, надо бы ещё и NPE забояться в 
Код

t.name.equals(this.name)

smile

Код

if (t.name != null)
    return t.name.equals(this.name);
else 
    return this.name == null;

Автор: Shaggie 18.10.2007, 10:34
Ээй, хватит! Так вы на этом участке скоро и OutOfMemoryException перехватывать будете!

А Null успешно ловится на проверке instanceof, так что это, право, лишнее.

Автор: Royan 18.10.2007, 11:05
Shaggie Справделивости ради OutOfMemoryError, а не exception... ну и я думаю никому не надо объяснять что error'ы никто не ловит, на то они и Error'ы

Автор: Shaggie 18.10.2007, 11:48
Не-а. Exception. И его можно перехватить и обработать. Столкнулся с ним при работе с картинками - распакованные в памяти jpeg занимают очень много места, стандартных 64 мегабайт кучи не хватает и вылетает Exception.

http://www.google.ru/search?q=java+outofmemoryexception&ie=utf-8&oe=utf-8&aq=t&rls=org.mozilla:ru:official&client=firefox-a

Автор: Royan 18.10.2007, 12:13
Shaggie, Еще разок в JavaAPI такого Exception'а нет, Он может появиться только если его кто то написал для своих никому неведомых целей, а гугл иногда говорит глупости

Автор: Shaggie 18.10.2007, 12:28
Так... а http://onesearch.sun.com/search/onesearch/index.jsp?charset=utf-8&col=all-filtered&qt=outofmemoryexception&y=0&x=0&cs=false&rt=true тоже глупости говорит?

Автор: chief39 18.10.2007, 12:32
Гы smile Вспомнили таки о NPE smile)

Автор: mindflyer 18.10.2007, 14:05
Цитата(Shaggie @  18.10.2007,  10:34 Найти цитируемый пост)
Ээй, хватит! Так вы на этом участке скоро и OutOfMemoryException перехватывать будете!
А Null успешно ловится на проверке instanceof, так что это, право, лишнее. 

В данном случае проверка необходима. instanceof делается для сравниваемого объекта, я же написал о равенству null поля объекта.

Автор: fixxer 18.10.2007, 14:34
Цитата(mindflyer @ 18.10.2007,  14:05)
Цитата(Shaggie @  18.10.2007,  10:34 Найти цитируемый пост)
Ээй, хватит! Так вы на этом участке скоро и OutOfMemoryException перехватывать будете!
А Null успешно ловится на проверке instanceof, так что это, право, лишнее. 

В данном случае проверка необходима. instanceof делается для сравниваемого объекта, я же написал о равенству null поля объекта.

И? В данном случае методы this.name не вызываются и NPE не случится. Все корректно отработает.

Автор: mindflyer 18.10.2007, 15:46
fixxer, Shaggie, для тех, кто не боится NPE:
Код

public class Test {
    private String name;
    public Test(String name) {
        this.name = name;
    }
    public boolean equals(Object obj) {
        if (this == obj) return true;
        if (!(obj instanceof Test)) return false;
        Test t = (Test)obj;
        return t.name.equals(this.name);
    }
    public static void main(String[] args) {
        Test t1 = new Test("");
        Test t2 = new Test(null);
        System.out.println(t2.equals(t1));
        System.out.println(t1.equals(t2));
    }
}

Автор: fixxer 18.10.2007, 16:06
Согласен.

Автор: Royan 18.10.2007, 16:36
Shaggie, Ты скажи, что пытаешься доказать? Что такой класс есть JavaAPI? Такого класса там нет! То что на сановых форумах поиск выводит OutOfMemoryException то это только лишь от того что многие его участники забывают, что OutOfMemory бывает только Error и пишут по привычке Exception

Автор: chief39 18.10.2007, 17:55
Код

public class MyClass{

private String str;

public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;
        MyClass tmp = (MyClass) o;
        return !(str != null ? !str.equals(tmp.str) : tmp.str != null);

    }
}


Разница в инстансоф и сравнении по getClass ну и.... нуллы - э то нуллы. Они есть и будут есть. Зачем такой абстрактный иквалз нужен?
Сферический конь в вакууме или принцесса, которая не какает.

Автор: mindflyer 19.10.2007, 08:57
Цитата(chief39 @  18.10.2007,  17:55 Найти цитируемый пост)
Разница в инстансоф и сравнении по getClass

Точно!  smile 
Во тема, блин smile 
Вопрос не по теме, но зачем сделано через отрицания в "return !(str != null ? !str.equals(tmp.str) : tmp.str != null);"? Исходя из каких-то соображений?

Автор: jsse 19.10.2007, 09:16
(offtopic) нигилизм наверное )

Добавлено @ 09:29
Если уже сильно нада сравнить, то я бы сделал так:

Код

public class MyClass {

    private String str;

    public String getString() {
          return str;
    }

    public boolean equals(Object o) {
        boolean ret = false;
        try {
            ((MyClass)o).equals(this); // если null, ClassCastException
            .... // OutOfMemoryError и т. д.
            if(str.equals(((MyClass)o).getString())) {
                 ret = true;
            }
        } catch (Exception e) {...}
        return ret;
    }
}

Автор: Shaggie 19.10.2007, 09:36
Royan, ок, принято. Свою ошибку признаю. Дома потестил - в самом деле Error вылетает, так что невнимательность на мне.

Автор: mindflyer 19.10.2007, 11:11
Цитата(jsse @  19.10.2007,  09:16 Найти цитируемый пост)
Если уже сильно нада сравнить, то я бы сделал так:

Механизм exception работает очень медленно по сравнению с обычными проверками (if, instanceof,...), не знаю какова разница в современной java, но про 1.4 видел в литературе высказывание о простых случаях (как рассматриваемый в этом топике) - "в десятки раз медленнее".

Автор: jsse 19.10.2007, 11:37
mindflyer, я описал ситуацию если нужно очень получить ответ, но не скорость выполнения ), хотя было бы интересно узнать факты и примеры где Exсeption отрабатует "в десятки раз медленнее"!

Автор: chief39 19.10.2007, 11:46
Цитата(mindflyer @  19.10.2007,  08:57 Найти цитируемый пост)
Вопрос не по теме, но зачем сделано через отрицания в "return !(str != null ? !str.equals(tmp.str) : tmp.str != null);"? Исходя из каких-то соображений?

Оптимизейшн. smile IDE предложила соптимизнуть - я согласился smile Было длиннее.

Цитата(mindflyer @  19.10.2007,  11:11 Найти цитируемый пост)

Механизм exception работает очень медленно по сравнению с обычными проверками (if, instanceof,...), не знаю какова разница в современной java, но про 1.4 видел в литературе высказывание о простых случаях (как рассматриваемый в этом топике) - "в десятки раз медленнее".

Вообще, эксепшн - это когда идёт что-то не так по нашей или чужой вине. "Соломка подстеленная на всякий случай".
А управление нормальным ходом программы с помощьюэксепшнов - плохой стиль.
Всё равно как вместо
if(obj == null){
  <blah-blah>
}

писать
try{
if(obj.getName == "ewe")
}catch(NullPointerException e){
  <blah-blah>
}
Суть ни капли не меняется, но читать удобнее и правильнее

Автор: jsse 19.10.2007, 11:56
chief39, еще хуже стиль когда в каждом сравнении типа if идет return

Автор: chief39 19.10.2007, 12:02
Цитата(jsse @  19.10.2007,  11:56 Найти цитируемый пост)
chief39, еще хуже стиль когда в каждом сравнении типа if идет return 

То есть?
Типа:
return x!=5;
да?

А что тут хуже? В том, что сразу "разбираются" тупиковые ветки алгоритма и бесповоротно "отсекаются" ретурнами? Для того, чтоб в конце блока случайно не наткнуться на давно забытое в начале.

То бишь, есть претензии по варианту с несколькими ретурнами? smile
Можно пояснить свою личную точку зрения? smile

Автор: jsse 19.10.2007, 12:11
chief39, обычно код одной ф-ции не занимает 5 строк ) Например сложно будет читать код в котором 10 return на 20 строк кода.

Автор: Shaggie 19.10.2007, 12:36
Цитата(jsse @  19.10.2007,  13:11 Найти цитируемый пост)
обычно код одной ф-ции не занимает 5 строк ) Например сложно будет читать код в котором 10 return на 20 строк кода

Ща пофантазирую... 10 return - это 10 различных (!) ситуаций, из-за которых выполнение функции может завершиться досрочно. Причём не все они выбрасывают исключение. Значит, надо для отслеживания некоторых ситуаций писать собственные исключения.

Если мы вместо 10 возвратов используем один большой трай-кетч блок, то сразу потеряем возможность отслежить причину выброшенного исключения, чтобы соответствующим образом отреагировать.

А если используем 10 разных трай-кетч блоков, причём вложим их так, чтобы при вылетании исключения выполнение метода завершалось с нужным нам результатом, то можно будет в паспорте в графе "национальность" вписывать "индус".

Предпочитаю честные возвраты.

Автор: jsse 19.10.2007, 13:11
Shaggie, хаха! посмеялся +1.  Я еще раз повторюсь - один из вариантов который я предложил, чтоб избежать ошибок - завернул в "Exception", но конечно, скорее всего, лучше было бы получить с помощью сравниния всевозможных ситуаций, и возможно, это правильней и производительней ) Всё равно как ни крути любая программа содежит минимум 3 ошибки. А на счет return в середине блока, даже в книге Шилдта и Нортона, которых стояли у истоков языка, не рекомендовали использовать возврат в середине функции. Да и как по мне, сложнее читать чужой код когда ищешь где функция должна вернуть тебе ответ, согласитесь гораздо проще увидеть что на входе(вызов функции) и что на выходе(возврат - return).

Автор: fixxer 19.10.2007, 13:16
Шилдт и Нортон попсовики-писатели, им все равно про что боянить Java, C++, .Net

Автор: Shaggie 19.10.2007, 13:20
В книге Джоша Блоха "Effective Java" в главе посвящённой equals он как раз не гнушается использовать возвраты точь в точь как chief39. А ведь товарищ - архитектор Java, его фамилию можно наблюдать в исходных кодах.

Каждому своё.

Автор: jsse 19.10.2007, 13:21
Что значит попсовики-писатели??

Добавлено через 4 минуты и 40 секунд
Shaggie, конечно - это то же что и обсуждать стиль написания программы. каждому своё, но есть стандарты.

Добавлено через 11 минут и 42 секунды
Shaggie, Книгу конечно полистаю "Effective Java: Programming Language Guide
 By Joshua Bloch"

Автор: jsse 19.10.2007, 13:39
Shaggie, где можно взять русский перевод?

Автор: mindflyer 19.10.2007, 14:28
Цитата(jsse @  19.10.2007,  13:11 Найти цитируемый пост)
А на счет return в середине блока, даже в книге Шилдта и Нортона, которых стояли у истоков языка, не рекомендовали использовать возврат в середине функции.

Как пишет Мартин Фаулер (и я с ним полностью согласен на основе своего опыта) идея одной точки выхода (избегание возвратов в разных местах функции) - очень эффективна в рамках концепции структурного программирования. Однако, java это уже ООП, в котором размер методов как правило невелик, и обычно гораздо проще понимать код, когда выход из функции происходит именно в тот момент (в той точке), когда становится ясно, что дальнейшая его работа уже не нужна. Потому лично я сторонник стиля, о котором пишет chief39

Автор: chief39 19.10.2007, 15:21
Цитата(jsse @  19.10.2007,  12:11 Найти цитируемый пост)
chief39, обычно код одной ф-ции не занимает 5 строк ) Например сложно будет читать код в котором 10 return на 20 строк кода.

Обычно методы(не функции! это важно в данном контексте) по сути и не требуют десяти ретурнов. Но требют по логике работы парочки throw. Т.к. могут быть неопределённости в процессе выполнения.
А супербольшие методы - это уже зло.

А вот методы, подобные иквалз и хэшкод - довольно специфичны по задаче своей.
Большинство методов работают с какими-то осмысленными крупнозернистыми операциями.
А такие сервис-методы имеют задачей "проверить все поля и сказать ДА или НЕТ".
Упомянутые недавно КА этим и занимаются. И лучше оптимизировать, дабы не танцевать вокруг всего объекта, если по первому пропери уже ясно что ОБЪЕКТ НЕ КАНАЕТ.
Джавасоздатели зачем-то сделали короткое замыкание в логических выражениях smile Неужто в этом тоже нет смысла?  smile 

По логике А:
"тэк, взяли объект, ага.. он налл - всё, до свиданья! Не налл? Аха.. проверяем дальше, что там в следующем проперти? ...."
По логике В:
"взяли. попробовали. налл. эксепшн. так. теперь пошли ловить эксепшн. ага, поймали, выходим."
Или ещё лучше:
"ага, налл.. запишем в переменную метода. так, пошли дальше, дальше, дальше, дальше... ага. конец. тааак... что там с переменной? ага.. не подошло ещё в саомм начале... тэээк-с.. начинаем выходить из метода".
Красиво, кто ж спорит  smile 

Автор: nornad 19.10.2007, 16:39
Имхо, дискуссия на тему throw/if-return не имеет смысла. Оба имеют свои пределы применения, обоими можно злоупотреблять. Не будете же вы все if заменять на throw. И наоборот. Так чего спорить-то? smile
Предпочитаете кидать эксепшены - кидайте. Если переборщите, сами услышите "запах" кода.  smile 
То же самое и в обратном направлении.

Автор: w1nd 21.10.2007, 03:35
Вот правильно реализованный метод equals()
Код
public class MyClass {

    private String string;

    public MyClass(String string) {
        if (string == null) {
            throw new IllegalArgumentException(...);
        }
        this.string = string;
    }

    public boolean equals(Object object) {
        return object != null && getClass() == object.getClass() && string.equals(object.string);
    }

}

Во-первых, метод equals() не должен породить NullPointerException, если аргумент == null. Читаем javadoc:
Цитата(javadoc)
For any non-null reference value x, x.equals(null) should return false.

Во-вторых, если вы используете instanceof вместо однозначной идентификации класса, вам стоит сделать метод equals() финальным. Потому что в ином случае наследник вашего класса может нарушить одно важное правило:
Цитата(javadoc)
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.

Автор: Shaggie 22.10.2007, 05:47
Цитата(jsse @  19.10.2007,  14:39 Найти цитируемый пост)
где можно взять русский перевод?

В инете - не знаю, а в книжном видел за 280 р., попробуй поискать. Чуть не купил... денег с собой не было. Красноярск.

Автор: LSD 22.10.2007, 08:56
Цитата(w1nd @  21.10.2007,  04:35 Найти цитируемый пост)
Во-вторых, если вы используете instanceof вместо однозначной идентификации класса, вам стоит сделать метод equals() финальным. Потому что в ином случае наследник вашего класса может нарушить одно важное правило:

Причем сами создатели JDK нарушают это правило smile

Добавлено через 4 минуты и 14 секунд
Хотя по мне, реализация через getClass() не самая лучшая.

Powered by Invision Power Board (http://www.invisionboard.com)
© Invision Power Services (http://www.invisionpower.com)