Issue #272

Improving error handling in DockableMenuItem. Unfortunately this required adding java.logging as a required module. That's not exactly a patch fix, but it should have been there anyways and it's part of Java, so not a big deal.

demo-single-app/src/basic/MainFrame.java

@@ -230,6 +230,7 @@ public MainFrame(File layoutFile) {
 		view.add(actionListenDock(explorer));
 		view.add(actionListenDock(output));
 		view.add(actionListenDock(fixedSize));
+		view.add(new DockableMenuItem("non-existent-dockable", "Does Not Exist"));
 		view.add(actionListenDock(propertiesDemoPanel));
 		view.add(new DockableMenuItem(() -> ((Dockable) alwaysDisplayed).getPersistentID(), ((Dockable) alwaysDisplayed).getTabText()));
 		view.add(changeText);

docking-api/src/ModernDocking/internal/DockingInternal.java

@@ -242,6 +242,10 @@ public DockableWrapper getWrapper(Dockable dockable) {
 		throw new DockableNotFoundException(dockable.getPersistentID());
 	}
 
+	public boolean hasDockable(String persistentID) {
+		return dockables.containsKey(persistentID);
+	}
+
 	/**
 	 * Find a dockable with the given persistent ID
 	 * @param persistentID persistent ID to search for

docking-multi-app/src/ModernDocking/app/DockableMenuItem.java

@@ -29,11 +29,15 @@ of this software and associated documentation files (the "Software"), to deal
 import java.awt.event.ActionEvent;
 import java.awt.event.ActionListener;
 import java.util.function.Supplier;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 /**
  * Special JCheckBoxMenuItem that handles updating the checkbox for us based on the docking state of the dockable
  */
 public class DockableMenuItem extends JCheckBoxMenuItem implements ActionListener {
+	private static final Logger logger = Logger.getLogger(DockableMenuItem.class.getPackageName());
+
 	/**
 	 * Persistent ID provider. Used when the persistent ID isn't known at compile time.
 	 * Used when this menu item is added to a menu (using addNotify)
@@ -102,18 +106,41 @@ public DockableMenuItem(DockingAPI docking, Supplier<String> persistentIDProvide
 	public void addNotify() {
 		super.addNotify();
 
+		// update the menu item, it's about to be displayed
+		DockingInternal internal = DockingInternal.get(docking);
+
+		String id = persistentIDProvider != null ? persistentIDProvider.get() : persistentID;
+
+		if (internal.hasDockable(id)) {
+			Dockable dockable = internal.getDockable(id);
+			setSelected(docking.isDocked(dockable));
+		}
+		else {
+			setVisible(false);
+			logger.log(Level.INFO, "Hiding DockableMenuItem for \"" + getText() + ".\" No dockable with persistentID '" + id + "' registered.");
+		}
+
 		// update the menu item, it's about to be displayed
 		Dockable dockable = DockingInternal.get(docking).getDockable(persistentIDProvider != null ? persistentIDProvider.get() : persistentID);
 		setSelected(docking.isDocked(dockable));
 	}
 
 	@Override
 	public void actionPerformed(ActionEvent e) {
-		Dockable dockable = DockingInternal.get(docking).getDockable(persistentIDProvider != null ? persistentIDProvider.get() : persistentID);
+		DockingInternal internal = DockingInternal.get(docking);
 
-		docking.display(dockable);
+		String id = persistentIDProvider != null ? persistentIDProvider.get() : persistentID;
 
-		// set this menu item to the state of the dockable, should be docked at this point
-		setSelected(docking.isDocked(dockable));
+		if (internal.hasDockable(id)) {
+			Dockable dockable = internal.getDockable(id);
+
+			docking.display(dockable);
+
+			// set this menu item to the state of the dockable, should be docked at this point
+			setSelected(docking.isDocked(dockable));
+		}
+		else {
+			logger.log(Level.SEVERE, "DockableMenuItem for \"" + getText() + "\" action failed. No dockable with persistentID '" + id + "' registered.");
+		}
 	}
 }

docking-multi-app/src/module-info.java

@@ -4,6 +4,7 @@
 module modern_docking.multi_app {
 	requires modern_docking.api;
 	requires java.desktop;
+    requires java.logging;
 
-	exports ModernDocking.app;
+    exports ModernDocking.app;
 }

docking-single-app/src/ModernDocking/app/DockableMenuItem.java

@@ -22,18 +22,23 @@ of this software and associated documentation files (the "Software"), to deal
 package ModernDocking.app;
 
 import ModernDocking.Dockable;
+import ModernDocking.api.DockingStateAPI;
 import ModernDocking.exception.DockableRegistrationFailureException;
 import ModernDocking.internal.DockingInternal;
 
 import javax.swing.*;
 import java.awt.event.ActionEvent;
 import java.awt.event.ActionListener;
 import java.util.function.Supplier;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 /**
  * Special JCheckBoxMenuItem that handles updating the checkbox for us based on the docking state of the dockable
  */
 public class DockableMenuItem extends JCheckBoxMenuItem implements ActionListener {
+	private static final Logger logger = Logger.getLogger(DockableMenuItem.class.getPackageName());
+
 	/**
 	 * Persistent ID provider. Used when the persistent ID isn't known at compile time.
 	 * Used when this menu item is added to a menu (using addNotify)
@@ -86,22 +91,36 @@ public void addNotify() {
 		super.addNotify();
 
 		// update the menu item, it's about to be displayed
-		try {
-			Dockable dockable = DockingInternal.get(Docking.getSingleInstance()).getDockable(persistentIDProvider != null ? persistentIDProvider.get() : persistentID);
+		DockingInternal internal = DockingInternal.get(Docking.getSingleInstance());
+
+		String id = persistentIDProvider != null ? persistentIDProvider.get() : persistentID;
+
+		if (internal.hasDockable(id)) {
+			Dockable dockable = internal.getDockable(id);
 			setSelected(Docking.isDocked(dockable));
 		}
-		catch (DockableRegistrationFailureException ignored) {
+		else {
 			setVisible(false);
+			logger.log(Level.INFO, "Hiding DockableMenuItem for \"" + getText() + ".\" No dockable with persistentID '" + id + "' registered.");
 		}
 	}
 
 	@Override
 	public void actionPerformed(ActionEvent e) {
-		Dockable dockable = DockingInternal.get(Docking.getSingleInstance()).getDockable(persistentIDProvider != null ? persistentIDProvider.get() : persistentID);
+		DockingInternal internal = DockingInternal.get(Docking.getSingleInstance());
+
+		String id = persistentIDProvider != null ? persistentIDProvider.get() : persistentID;
 
-		Docking.display(dockable);
+		if (internal.hasDockable(id)) {
+			Dockable dockable = internal.getDockable(id);
 
-		// set this menu item to the state of the dockable, should be docked at this point
-		setSelected(Docking.isDocked(dockable));
+			Docking.display(dockable);
+
+			// set this menu item to the state of the dockable, should be docked at this point
+			setSelected(Docking.isDocked(dockable));
+		}
+		else {
+			logger.log(Level.SEVERE, "DockableMenuItem for \"" + getText() + "\" action failed. No dockable with persistentID '" + id + "' registered.");
+		}
 	}
 }

docking-single-app/src/module-info.java

@@ -4,6 +4,7 @@
 module modern_docking.single_app {
 	requires modern_docking.api;
 	requires java.desktop;
+    requires java.logging;
 
-	exports ModernDocking.app;
+    exports ModernDocking.app;
 }