[GitHub] [netbeans] timboudreau commented on a change in pull request #2232: Make it easy to create dynamically updated ProxyLookup instances without subclassing

[GitBox <gi...@apache.org>]

timboudreau commented on a change in pull request #2232:
URL: https://github.com/apache/netbeans/pull/2232#discussion_r462085779



##########
File path: platform/openide.util.lookup/src/org/openide/util/lookup/ProxyLookup.java
##########
@@ -89,7 +110,96 @@ public synchronized String toString() {
         }
         return map.keySet();
     }
-    
+
+    /**
+     * A controller which allows the set of lookups being proxied to be
+     * set dynamically.
+     *
+     * @since 8.43
+     */
+    public static final class Controller {
+
+        private BiConsumer<? super Executor, ? super Lookup[]> consumer;
+
+        /**
+         * Create a controller with an initial set of lookups that will be
+         * proxied by a {@link ProxyLookup} this controller is passed to the
+         * constructor of.
+         * @param lookups An array of lookups
+         */
+        public Controller(Lookup... lookups) {
+            consumer = new InitialConsumer(lookups);
+        }
+
+        /**
+         * Create a controller with an initially empty set of lookups to
+         * proxy to.
+         */
+        public Controller() {
+            consumer = new InitialConsumer();
+        }
+
+        /**
+         * Set the lookups on the {@link ProxyLookup} this controller controls;
+         * if called before that ProxyLookup has been created, the lookup
+         * contents will be set during that ProxyLookup's constructor as if you
+         * had passed them to the constructor directly, but the executor
+         * parameter will be ignored (nothing will be listening to it while it's
+         * in its constructor anyway).
+         *
+         * @param exe An executor to notify in
+         * @param lookups An array of Lookups to be proxied
+         */
+        public synchronized void setLookups(Executor exe, Lookup... lookups) {
+            consumer.accept(exe, lookups);

Review comment:
       Throw `IllegalArgumentException`, maybe.  It is a bad habit to abuse `NullPointerException` for argument checking - when you see a `NullPointerException`, that should mean the JVM threw it because it was asked to do something that is impossible with a null.  One case is a bad *argument* that was checked (bug in the caller), the other is a bug in the callee (at least, failure to check the input) - if you use `NullPointerException` for both cases, you lose the ability to assume `IllegalArgumentException` means *my code is probably wrong, look at that first* and `NullPointerException` means *library code may have a problem, maybe look at that first* - and that makes debugging things faster.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists