From 0aca375f214c7edc2395fca0d88b36d355e3831d Mon Sep 17 00:00:00 2001
From: Jules Kerssemakers <jules.kerssemakers@gmail.com>
Date: Tue, 2 Jan 2024 13:30:03 +0100
Subject: [PATCH] review #83: stronger non-null on Piano.addListener.

---
 app/src/main/java/com/nicobrailo/pianoli/Piano.java  | 12 +++++++-----
 .../com/nicobrailo/pianoli/PianoListenerTest.java    |  3 ++-
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/app/src/main/java/com/nicobrailo/pianoli/Piano.java b/app/src/main/java/com/nicobrailo/pianoli/Piano.java
index a64f1bc..21be151 100644
--- a/app/src/main/java/com/nicobrailo/pianoli/Piano.java
+++ b/app/src/main/java/com/nicobrailo/pianoli/Piano.java
@@ -1,10 +1,12 @@
 package com.nicobrailo.pianoli;
 
 import android.util.Log;
+import androidx.annotation.NonNull;
 
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Objects;
 
 /**
  * Backing model / state of our virtual piano keyboard.
@@ -140,11 +142,11 @@ private boolean isOutOfRange(int key_idx) {
      * @return Per the {@link java.util.Collection#add(Object)} contract, <code>true</code> if the listener list changed as a result of this add,
      *          <code>false</code> if it was already subscribed.
      */
-    public boolean addListener(PianoListener l) {
-        if (l != null  // avoid NullPointerExceptions on notify
-                && !listeners.contains(l)) { // don't double-add listeners, to avoid double-triggers
-            listeners.add(l);
-            return true;
+    public boolean addListener(@NonNull PianoListener l) {
+        Objects.requireNonNull(l, "Listeners must not be null to avoid NullPointerExceptions on notify");
+
+        if (!listeners.contains(l)) { // don't double-add listeners, to avoid double-triggers
+            return listeners.add(l);
         }
         return false;
     }
diff --git a/app/src/test/java/com/nicobrailo/pianoli/PianoListenerTest.java b/app/src/test/java/com/nicobrailo/pianoli/PianoListenerTest.java
index 7e1283a..fe6cea6 100644
--- a/app/src/test/java/com/nicobrailo/pianoli/PianoListenerTest.java
+++ b/app/src/test/java/com/nicobrailo/pianoli/PianoListenerTest.java
@@ -77,7 +77,8 @@ public void doubleRemoval() {
 
     @Test
     public void nullSafeAdd() {
-        assertFalse(piano.addListener(null), "null-listener should be silently accepted, but not do anything");
+        //noinspection DataFlowIssue // intentionally violating @NonNull to test handling.
+        assertThrows(NullPointerException.class, () -> piano.addListener(null), "null-listener should be silently accepted, but not do anything");
 
         assertDoesNotThrow(() -> piano.doKeyDown(0),
                 "after adding a null-listener, notification should not explode");