Skip to content

Commit

Permalink
PersonCard: remove #equals(object)
Browse files Browse the repository at this point in the history
This method implements a very weak notion of equivalence, where as long
as the PersonCard's person and id are equal, the object is equal.
However, such a weak notion of equivalence goes against the spirit of
the UI architecture, which states that all UiParts represent a distinct
part of the UI, and more importantly, wraps a subgraph of the JavaFX
scene graph (accessible via UiPart#getRoot()). As such, just because two
PersonCards A and B happen to contain the same ID and person value,
doesn't mean they are "equal", because they may be holding different
JavaFX scene graph objects. Any JavaFX-related operations on PersonCard
A may give a different result to PersonCard B.

Furthermore, this implementation of PersonCard#equals(Object) isn't
actually used, not even by test code. It was introduced in baa5988
(PersonListPanelHandle: Update methods, 2017-07-14) so that one could
do the following to assert that a certain PersonCard was selected:

    private void assertCardSelected(Index index) {
        PersonCard selectedCard = getPersonListPanel().getSelectedCard().get();
        assertEquals(getPersonListPanel().getCard(index.getZeroBased()), selectedCard);
    }

where the assertEquals() would then call PersonCard#equals(Object).

However, since 04a675f (PersonListPanelHandle: Update
getSelectedCard(), 2017-07-20) this pattern of code has been (rightly)
removed, with test code preferring to compare what is actually displayed
on the PersonCard, rather than using PersonCard#equals(Object):

    /**
     * Asserts that {@code actualCard} displays the same values as {@code expectedCard}.
     */
    public static void assertCardEquals(PersonCardHandle expectedCard, PersonCardHandle actualCard) {
        assertEquals(expectedCard.getId(), actualCard.getId());
        assertEquals(expectedCard.getAddress(), actualCard.getAddress());
        assertEquals(expectedCard.getEmail(), actualCard.getEmail());
        assertEquals(expectedCard.getName(), actualCard.getName());
        assertEquals(expectedCard.getPhone(), actualCard.getPhone());
        assertEquals(expectedCard.getTags(), actualCard.getTags());
    }

PersonCard#equals(Object) was, however, not removed then. So let's
remove it now.
  • Loading branch information
pyokagan authored and damithc committed Aug 4, 2023
1 parent 15b62ac commit e9f1ca7
Showing 1 changed file with 0 additions and 16 deletions.
16 changes: 0 additions & 16 deletions src/main/java/seedu/address/ui/PersonCard.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,20 +56,4 @@ public PersonCard(Person person, int displayedIndex) {
.sorted(Comparator.comparing(tag -> tag.tagName))
.forEach(tag -> tags.getChildren().add(new Label(tag.tagName)));
}

@Override
public boolean equals(Object other) {
if (other == this) {
return true;
}

// instanceof handles nulls
if (!(other instanceof PersonCard)) {
return false;
}

PersonCard card = (PersonCard) other;
return id.getText().equals(card.id.getText())
&& person.equals(card.person);
}
}

0 comments on commit e9f1ca7

Please sign in to comment.