StashPullRequestComment: Rewrite Comparable interface implementation

Define hashCode() and equals() to ensure consistent behavior and fix
FindBugs warnings. Remove FindBugs suppressions.

Treat null as Integer.MIN_VALUE in hashCode(), as that comment ID is not
expected to be found in actual server responses.

Make equals() and compareTo() compare ID only. Comparing text is not
needed because comparison is only used for comments in the same pull
request. Each comment in the pull request should have an ID unique within
that pull request.

Add extensive unit tests.

src/main/java/stashpullrequestbuilder/stashpullrequestbuilder/stash/StashPullRequestComment.java

@@ -1,11 +1,10 @@
 package stashpullrequestbuilder.stashpullrequestbuilder.stash;
 
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import java.util.Objects;
 import org.codehaus.jackson.annotate.JsonIgnoreProperties;
 import org.codehaus.jackson.annotate.JsonProperty;
 
 /** Created by Nathan McCarthy */
-@SuppressFBWarnings("EQ_COMPARETO_USE_OBJECT_EQUALS")
 @JsonIgnoreProperties(ignoreUnknown = true)
 public class StashPullRequestComment implements Comparable<StashPullRequestComment> {
 
@@ -31,13 +30,24 @@ public void setText(String text) {
   }
 
   @Override
-  public int compareTo(StashPullRequestComment target) {
-    if (this.getCommentId() > target.getCommentId()) {
-      return 1;
-    } else if (this.getCommentId().equals(target.getCommentId())) {
-      return 0;
-    } else {
-      return -1;
+  public int hashCode() {
+    return (commentId == null) ? Integer.MIN_VALUE : commentId;
+  }
+
+  @Override
+  public boolean equals(Object o) {
+    if (!(o instanceof StashPullRequestComment)) {
+      return false;
     }
+
+    StashPullRequestComment other = (StashPullRequestComment) o;
+
+    return Objects.equals(this.commentId, other.commentId);
+  }
+
+  @Override
+  public int compareTo(StashPullRequestComment other) {
+    return Objects.compare(
+        this.commentId, other.commentId, (Integer a, Integer b) -> a.compareTo(b));
   }
 }

src/test/java/stashpullrequestbuilder/stashpullrequestbuilder/stash/StashPullRequestCommentTest.java

@@ -0,0 +1,155 @@
+package stashpullrequestbuilder.stashpullrequestbuilder.stash;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.lessThan;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+/** Unit tests for StashPullRequestComment */
+public class StashPullRequestCommentTest {
+
+  @Rule public ExpectedException expectedException = ExpectedException.none();
+
+  @Test
+  public void accessors_work() {
+    StashPullRequestComment comment = new StashPullRequestComment();
+    comment.setCommentId(42);
+    comment.setText("Build Started");
+    assertThat(comment.getCommentId(), is(equalTo(42)));
+    assertThat(comment.getText(), is(equalTo("Build Started")));
+  }
+
+  @Test
+  public void hashCode_determined_by_commentId() {
+    StashPullRequestComment comment = new StashPullRequestComment();
+    assertThat(comment.hashCode(), is(equalTo(Integer.MIN_VALUE)));
+
+    comment.setCommentId(Integer.MAX_VALUE);
+    assertThat(comment.hashCode(), is(equalTo(Integer.MAX_VALUE)));
+
+    comment.setCommentId(123);
+    assertThat(comment.hashCode(), is(equalTo(123)));
+
+    comment.setCommentId(-1000);
+    assertThat(comment.hashCode(), is(equalTo(-1000)));
+
+    comment.setText("42");
+    assertThat(comment.hashCode(), is(equalTo(-1000)));
+  }
+
+  @Test
+  public void comment_unequal_to_null() {
+    StashPullRequestComment comment = new StashPullRequestComment();
+
+    assertThat(comment.equals(null), is(equalTo(false)));
+  }
+
+  @Test
+  public void comment_with_id_equal_to_itself() {
+    StashPullRequestComment comment = new StashPullRequestComment();
+    comment.setCommentId(1);
+
+    assertThat(comment.equals(comment), is(equalTo(true)));
+    assertThat(comment.compareTo(comment), is(equalTo(0)));
+  }
+
+  @Test
+  public void comment_with_null_id_equal_to_itself() {
+    StashPullRequestComment comment = new StashPullRequestComment();
+
+    assertThat(comment.equals(comment), is(equalTo(true)));
+    assertThat(comment.compareTo(comment), is(equalTo(0)));
+  }
+
+  @Test
+  public void comment_compareTo_null_throws() {
+    StashPullRequestComment comment = new StashPullRequestComment();
+
+    expectedException.expect(NullPointerException.class);
+    comment.compareTo(null);
+  }
+
+  @Test
+  public void comments_with_same_id_are_equal() {
+    StashPullRequestComment comment1 = new StashPullRequestComment();
+    comment1.setCommentId(1);
+    comment1.setText("1");
+
+    StashPullRequestComment comment2 = new StashPullRequestComment();
+    comment2.setCommentId(1);
+    comment2.setText("2");
+
+    assertThat(comment1.equals(comment2), is(equalTo(true)));
+    assertThat(comment2.equals(comment1), is(equalTo(true)));
+    assertThat(comment1.compareTo(comment2), is(equalTo(0)));
+    assertThat(comment2.compareTo(comment1), is(equalTo(0)));
+  }
+
+  @Test
+  public void comments_with_null_id_are_equal() {
+    StashPullRequestComment comment1 = new StashPullRequestComment();
+    comment1.setText("1");
+
+    StashPullRequestComment comment2 = new StashPullRequestComment();
+    comment2.setText("2");
+
+    assertThat(comment1.equals(comment2), is(equalTo(true)));
+    assertThat(comment2.equals(comment1), is(equalTo(true)));
+    assertThat(comment1.compareTo(comment2), is(equalTo(0)));
+    assertThat(comment2.compareTo(comment1), is(equalTo(0)));
+  }
+
+  @Test
+  public void comments_with_different_id_are_unequal() {
+    StashPullRequestComment comment1 = new StashPullRequestComment();
+    comment1.setCommentId(1);
+    comment1.setText("1");
+
+    StashPullRequestComment comment2 = new StashPullRequestComment();
+    comment2.setCommentId(2);
+    comment2.setText("1");
+
+    assertThat(comment1.equals(comment2), is(equalTo(false)));
+    assertThat(comment2.equals(comment1), is(equalTo(false)));
+    assertThat(comment1.compareTo(comment2), is(lessThan(0)));
+    assertThat(comment2.compareTo(comment1), is(greaterThan(0)));
+  }
+
+  @Test
+  public void comment_with_nonnull_id_unequal_to_comment_with_null_id() {
+    StashPullRequestComment comment1 = new StashPullRequestComment();
+    comment1.setCommentId(1);
+
+    StashPullRequestComment comment2 = new StashPullRequestComment();
+
+    assertThat(comment1.equals(comment2), is(equalTo(false)));
+    assertThat(comment2.equals(comment1), is(equalTo(false)));
+  }
+
+  @Test
+  public void comparison_throws_for_null_id_in_caller() {
+    StashPullRequestComment comment1 = new StashPullRequestComment();
+
+    StashPullRequestComment comment2 = new StashPullRequestComment();
+    comment2.setCommentId(2);
+
+    expectedException.expect(NullPointerException.class);
+    comment1.compareTo(comment2);
+  }
+
+  @Test
+  public void comparison_throws_for_null_id_in_argument() {
+    StashPullRequestComment comment1 = new StashPullRequestComment();
+    comment1.setCommentId(1);
+
+    StashPullRequestComment comment2 = new StashPullRequestComment();
+
+    expectedException.expect(NullPointerException.class);
+    comment1.compareTo(comment2);
+  }
+}