Skip to content

Commit

Permalink
review #83: stronger non-null on Piano.addListener.
Browse files Browse the repository at this point in the history
  • Loading branch information
Jules Kerssemakers committed Jan 2, 2024
1 parent 5f891bc commit 0aca375
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 6 deletions.
12 changes: 7 additions & 5 deletions app/src/main/java/com/nicobrailo/pianoli/Piano.java
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down

0 comments on commit 0aca375

Please sign in to comment.