diff --git a/app/src/main/java/com/nicobrailo/pianoli/AppConfigTrigger.java b/app/src/main/java/com/nicobrailo/pianoli/AppConfigTrigger.java index 12b88bf..4571aba 100644 --- a/app/src/main/java/com/nicobrailo/pianoli/AppConfigTrigger.java +++ b/app/src/main/java/com/nicobrailo/pianoli/AppConfigTrigger.java @@ -1,6 +1,7 @@ package com.nicobrailo.pianoli; import android.util.Log; +import androidx.annotation.NonNull; import java.util.Arrays; import java.util.HashSet; @@ -44,6 +45,13 @@ class AppConfigTrigger implements PianoListener { *

*/ private final Set pressedConfigKeys = new HashSet<>(); + + /** + * User frustration tracker: how badly are they failing to open the config? + * + * @see #cb + * @see #setConfigRequestCallback(AppConfigCallback) + */ private TooltipReminder tooltipReminder; /** @@ -52,7 +60,10 @@ class AppConfigTrigger implements PianoListener { private int nextExpectedKey; /** - * Our "upstream", who knows enougjh about global app context to actually do stuff. + * Our "upstream", who knows enough about global app context to actually do stuff. + * + * @see #tooltipReminder + * @see #setConfigRequestCallback(AppConfigCallback) */ private AppConfigCallback cb = null; @@ -60,9 +71,18 @@ class AppConfigTrigger implements PianoListener { nextExpectedKey = calculateNextExpectedKey(); } - void setConfigRequestCallback(AppConfigCallback cb) { + /** + * Dependency injection for context-handling stuff: switching to settings activity, and showing toasts. + * + *

+ * Since this callback is required for this trigger to do anything at all, it would have been preferable + * to require this is provided in the constructor. Alas, the way we initialise our upstream PianoCanvas + * via XML definition precludes this. + *

+ */ + void setConfigRequestCallback(@NonNull AppConfigCallback cb) { this.cb = cb; - tooltipReminder = new TooltipReminder(cb); + this.tooltipReminder = new TooltipReminder(cb); } /** @@ -144,9 +164,7 @@ public void onKeyDown(int keyIdx) { // Sequence complete! reset(); // clear it so it's no longer counted as in-progress. // Open Sesame! - if (cb != null) { - cb.requestConfig(); - } + cb.requestConfig(); } else { nextExpectedKey = calculateNextExpectedKey(); } 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, true if the listener list changed as a result of this add, * false 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/main/java/com/nicobrailo/pianoli/PianoCanvas.java b/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java index 9c6204a..c70a23a 100644 --- a/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java +++ b/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java @@ -126,7 +126,12 @@ public void reInitPiano(Context context, String prefSoundset) { Log.i("PianOli::PianoCanvas", "re-initialising Piano - DONE"); } - public void setConfigRequestCallback(AppConfigTrigger.AppConfigCallback cb) { + /** + * Dependency injection for context-handling stuff: switching to settings activity, and showing toasts. + * + * @see AppConfigTrigger#setConfigRequestCallback(AppConfigTrigger.AppConfigCallback) + */ + public void setConfigRequestCallback(@NonNull AppConfigTrigger.AppConfigCallback cb) { this.appConfigTrigger.setConfigRequestCallback(cb); } diff --git a/app/src/test/java/com/nicobrailo/pianoli/PianoListenerTest.java b/app/src/test/java/com/nicobrailo/pianoli/PianoListenerTest.java index 7e1283a..d1d1525 100644 --- a/app/src/test/java/com/nicobrailo/pianoli/PianoListenerTest.java +++ b/app/src/test/java/com/nicobrailo/pianoli/PianoListenerTest.java @@ -77,12 +77,13 @@ 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 not be accepted"); assertDoesNotThrow(() -> piano.doKeyDown(0), - "after adding a null-listener, notification should not explode"); + "after adding a null-listener, listener notification should not explode"); assertDoesNotThrow(() -> piano.doKeyUp(0), - "after adding a null-listener, notification should not explode"); + "after adding a null-listener, listener notification should not explode"); } @Test