On Fri, 9 Dec 2022 20:11:24 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> I suggest the following method: >> >> >> @Override >> public boolean equals(Object obj) { >> if (this == obj) { >> return true; >> } >> if (!(obj instanceof List<?> otherList)) { >> return false; >> } >> if (size() != otherList.size()) { >> return false; >> } >> ListIterator<E> e1 = listIterator(); >> ListIterator<?> e2 = otherList.listIterator(); >> while (e1.hasNext() && e2.hasNext()) { >> if (!Objects.equals(e1.next(), e2.next())) { >> return false; >> } >> } >> return true; >> } >> >> >> Technically, there's no need to check `hasNext` on both since they have the >> same length, but it doesn't matter. >> >> I also don't like the names `e1` and `e2` for list iterators :) > > There is a lot to not like about this method. Especially because apparently > you can pass in a `List` to this equals method that is for a > `ReadOnlyListProperty`. How is a list equal to a property? What sense does > it make to do this: > > ReadOnlyListProperty<String> x; > > x.equals(List.of("a", "b", "c")); > > Shouldn't that be: > > x.get().equals(List.of("a", "b", "c")); > > ? I was just going to write that now that I look at it, this is a list comparison according to `List::equals`. No need to redo the whole logic.. About the validity of the comparison, `ReadOnlyListProperty` also implements `List`: ReadOnlyListProperty<E> extends ListExpression<E> ListExpression<E> implements ObservableListValue<E> ObservableListValue<E> extends ObservableObjectValue<ObservableList<E>>, ObservableList<E> ObservableList<E> extends List<E>, Observable So... it's also a list. ------------- PR: https://git.openjdk.org/jfx/pull/969