Dexter icon indicating copy to clipboard operation
Dexter copied to clipboard

Assert not null listeners are being passed to Dexter

Open Serchinastico opened this issue 8 years ago • 2 comments

We should be checking that so that we don't throw NPEs

Serchinastico avatar Mar 08 '17 10:03 Serchinastico

@Serchinastico could you elaborate a bit more? What you had in mind when you created this issue? I have a patch that protect ourselves from a null PermissionRequestErrorListener like this:

diff --git a/dexter/src/main/java/com/karumi/dexter/Dexter.java b/dexter/src/main/java/com/karumi/dexter/Dexter.java
index 42ff146..70cfeb7 100644
--- a/dexter/src/main/java/com/karumi/dexter/Dexter.java
+++ b/dexter/src/main/java/com/karumi/dexter/Dexter.java
@@ -89,7 +89,9 @@ public final class Dexter
   }
 
   @Override public DexterBuilder withErrorListener(PermissionRequestErrorListener errorListener) {
-    this.errorListener = errorListener;
+    if (errorListener != null) {
+      this.errorListener = errorListener;
+    }
     return this;
   }
 

This will protect us from a null assignation for the error listener, but I'm not sure if that is what this issue aimed to describe.

untalfranfernandez avatar Jul 21 '17 13:07 untalfranfernandez

Yes, that's what (I think) I had in mind, don't forget we have several other listeners, not only the error one: https://github.com/Karumi/Dexter/blob/master/dexter/src/main/java/com/karumi/dexter/Dexter.java#L60 It might be a good idea to report back to the user when they try to do something like this, maybe adding another possible value to https://github.com/Karumi/Dexter/blob/master/dexter/src/main/java/com/karumi/dexter/listener/DexterError.java and sending it back to the error listener (if not null 😅 ).

Anyways, just protecting us from NPEs is good enough

Serchinastico avatar Jul 21 '17 14:07 Serchinastico