ttools: ConfigException is now checked, not unchecked

Reparent ConfigException to inherit from Exception not RuntimeException. It's really necessary to have it as a checked exception to make it clear when you have to handle it. It was only unchecked before because I was in a tearing hurry when I put did it in the first place. Added some code in topcat to behave sensibly when such an exception is caught, but this is not much tested so far; currently these are not often thrown.
Starlink · Feb 26, 2015 · 000359f · 000359f
1 parent 6c4a3be
commit 000359f
Show file tree

Hide file tree

Showing 23 changed files with 151 additions and 73 deletions.
diff --git a/topcat/src/main/uk/ac/starlink/topcat/plot2/BasicCoordLayerControl.java b/topcat/src/main/uk/ac/starlink/topcat/plot2/BasicCoordLayerControl.java
@@ -6,14 +6,13 @@
 import java.awt.event.ItemListener;
 import java.util.Map;
 import javax.swing.Box;
+import javax.swing.ComboBoxModel;
 import javax.swing.JComboBox;
 import javax.swing.JComponent;
-import javax.swing.ComboBoxModel;
 import uk.ac.starlink.topcat.LineBox;
 import uk.ac.starlink.topcat.RowSubset;
 import uk.ac.starlink.topcat.TablesListComboBox;
 import uk.ac.starlink.topcat.TopcatModel;
-import uk.ac.starlink.ttools.plot.Style;
 import uk.ac.starlink.ttools.plot2.DataGeom;
 import uk.ac.starlink.ttools.plot2.LegendEntry;
 import uk.ac.starlink.ttools.plot2.PlotLayer;
@@ -38,6 +37,7 @@ public class BasicCoordLayerControl extends ConfigControl
     private final JComboBox subsetSelector_;
     private final ComboBoxModel dummyComboBoxModel_;
     private final ReportLogger reportLogger_;
+    private final ConfigStyler styler_;
     private TopcatModel tcModel_;
 
     /**
@@ -53,6 +53,7 @@ public BasicCoordLayerControl( Plotter<?> plotter,
         plotter_ = plotter;
         coordPanel_ = coordPanel;
         reportLogger_ = new ReportLogger( this );
+        styler_ = new ConfigStyler( coordPanel_.getComponent() );
 
         /* Create data selection components. */
         tableSelector_ = new TablesListComboBox();
@@ -106,7 +107,8 @@ public PlotLayer[] getPlotLayers() {
         DataGeom geom = coordPanel_.getDataGeom();
         DataSpec dataSpec = new GuiDataSpec( tcModel_, subset, coordContents );
         ConfigMap config = getConfig();
-        PlotLayer layer = createLayer( plotter_, geom, dataSpec, config );
+        PlotLayer layer =
+            styler_.createLayer( plotter_, geom, dataSpec, config );
         return layer == null ? new PlotLayer[ 0 ] : new PlotLayer[] { layer };
     }
 
@@ -163,19 +165,4 @@ protected void tableChanged( TopcatModel tcModel ) {
         }
         subsetSelector_.setModel( subselModel );
     }
-
-    /**
-     * Creates a new layer from a plotter.
-     *
-     * @param  plotter  plotter
-     * @param  geom  data geom
-     * @param  dataSpec   data spec
-     * @param  config   style configuration
-     */
-    private static <S extends Style>
-            PlotLayer createLayer( Plotter<S> plotter, DataGeom geom,
-                                   DataSpec dataSpec, ConfigMap config ) {
-        S style = plotter.createStyle( config );
-        return plotter.createLayer( geom, dataSpec, style );
-    }
 }
diff --git a/topcat/src/main/uk/ac/starlink/topcat/plot2/ConfigStyler.java b/topcat/src/main/uk/ac/starlink/topcat/plot2/ConfigStyler.java
@@ -0,0 +1,78 @@
+package uk.ac.starlink.topcat.plot2;
+
+import java.awt.Component;
+import javax.swing.JOptionPane;
+import uk.ac.starlink.ttools.plot.Style;
+import uk.ac.starlink.ttools.plot2.DataGeom;
+import uk.ac.starlink.ttools.plot2.PlotLayer;
+import uk.ac.starlink.ttools.plot2.Plotter;
+import uk.ac.starlink.ttools.plot2.config.ConfigException;
+import uk.ac.starlink.ttools.plot2.config.ConfigMap;
+import uk.ac.starlink.ttools.plot2.data.DataSpec;
+
+/**
+ * Manages creation of PlotLayers from Plotters by turning ConfigMaps into
+ * appropriate Style instances.
+ * This would be just a case of calling the relevant Plotter method,
+ * except that method can throw a ConfigException, and we have to manage
+ * behaviour in the case that that happens.
+ *
+ * This is currently done by popping up a dialogue window the first time the
+ * error occurs.
+ * 
+ * @author   Mark Taylor
+ * @since    25 Feb 2015
+ */
+public class ConfigStyler {
+
+    private final Component parent_;
+    private boolean lastFailed_;
+
+    /**
+     * Constructor.
+     *
+     * @param   parent   parent component for dialogue windows
+     */
+    public ConfigStyler( Component parent ) {
+        parent_ = parent;
+    }
+
+    /**
+     * Creates a new layer from a plotter.
+     *
+     * @param  plotter  plotter
+     * @param  geom  data geom
+     * @param  dataSpec   data spec
+     * @param  config   style configuration
+     * @return   layer, or null in case of failure
+     */
+    public <S extends Style> PlotLayer createLayer( Plotter<S> plotter,
+                                                    DataGeom geom,
+                                                    DataSpec dataSpec,
+                                                    ConfigMap config ) {
+        S style;
+        try {
+            style = plotter.createStyle( config );
+            lastFailed_ = false;
+        }
+        catch ( ConfigException e ) {
+            if ( ! lastFailed_ ) {
+                String name = e.getConfigKey().getMeta().getLongName();
+                String[] msg = new String[] { name + ": ", e.getMessage() };
+                JOptionPane.showMessageDialog( parent_, msg, name + " Error",
+                                               JOptionPane.ERROR_MESSAGE );
+                lastFailed_ = true;
+            }
+            return null;
+        }
+        try {
+            S dupStyle = plotter.createStyle( config );
+            assert style.equals( dupStyle );
+            assert style.hashCode() == dupStyle.hashCode();
+        }
+        catch ( ConfigException e ) {
+            assert false : "Shouldn't happen";
+        }
+        return plotter.createLayer( geom, dataSpec, style );
+    }
+}
diff --git a/topcat/src/main/uk/ac/starlink/topcat/plot2/FormControl.java b/topcat/src/main/uk/ac/starlink/topcat/plot2/FormControl.java
@@ -38,6 +38,7 @@ public abstract class FormControl implements Control {
     private SubsetConfigManager subManager_;
     private FormStylePanel stylePanel_;
     private JComponent panel_;
+    private ConfigStyler styler_;
     private ReportPanel reportPanel_;
 
     /**
@@ -88,6 +89,20 @@ public JComponent getPanel() {
         return panel_;
     }
 
+    /**
+     * Returns this component's ConfigStyler.
+     *
+     * @return  config styler
+     */
+    private synchronized ConfigStyler getStyler() {
+
+        /* Constructed lazily because it needs the component. */
+        if ( styler_ == null ) {
+            styler_ = new ConfigStyler( getPanel() );
+        }
+        return styler_;
+    }
+
     /**
      * Returns the data and metadata for the additional coordinates
      * entered by the user in this control.
@@ -185,33 +200,13 @@ public FormStylePanel getStylePanel() {
      *                   set up for the given subset
      * @param  subset   row subset in the current table for which the
      *                  layer is to be plotted
-     * @return   new plot layer
+     * @return   new plot layer, may be null in case of incorrect GUI config
      */
     public PlotLayer createLayer( DataGeom geom, DataSpec dataSpec,
                                   RowSubset subset ) {
-        return createLayer( getPlotter(), geom, dataSpec, subset );
-    }
-
-    /**
-     * Creates a plot layer.
-     *
-     * @param  geom  data position geometry
-     * @param  dataSpec  data specification, which must contain any data
-     *                   required by this control's extra coords and be
-     *                   set up for the given subset
-     * @param  subset   row subset in the current table for which the
-     *                  layer is to be plotted
-     * @param   new plot layer
-     */
-    private <S extends Style>
-            PlotLayer createLayer( Plotter<S> plotter, DataGeom geom,
-                                   DataSpec dataSpec, RowSubset subset ) {
         ConfigMap config = stylePanel_.getConfig( subset );
         config.putAll( getExtraConfig() );
-        S style = plotter.createStyle( config );
-        assert style.equals( plotter.createStyle( config ) );
-        assert style.hashCode() == plotter.createStyle( config ).hashCode();
-        return plotter.createLayer( geom, dataSpec, style );
+        return getStyler().createLayer( getPlotter(), geom, dataSpec, config );
     }
 
     /**

diff --git a/topcat/src/main/uk/ac/starlink/topcat/plot2/FormLayerControl.java b/topcat/src/main/uk/ac/starlink/topcat/plot2/FormLayerControl.java
@@ -193,7 +193,10 @@ public PlotLayer[] getPlotLayers() {
                         PlotUtil.arrayConcat( posContents, extraContents );
                     DataSpec dspec =
                         new GuiDataSpec( tcModel_, subset, contents );
-                    layerList.add( fc.createLayer( geom, dspec, subset ) );
+                    PlotLayer layer = fc.createLayer( geom, dspec, subset );
+                    if ( layer != null ) {
+                        layerList.add( layer );
+                    }
                 }
             }
         }
@@ -244,10 +247,12 @@ public void submitReports( Map<LayerId,ReportMap> reports ) {
                         DataSpec dspec =
                             new GuiDataSpec( tcModel_, rset, contents );
                         PlotLayer layer = fc.createLayer( geom, dspec, rset );
-                        ReportMap report =
-                            reports.get( LayerId.createLayerId( layer ) );
-                        if ( report != null ) {
-                            sreports.put( rset, report );
+                        if ( layer != null ) {
+                            ReportMap report =
+                                reports.get( LayerId.createLayerId( layer ) );
+                            if ( report != null ) {
+                                sreports.put( rset, report );
+                            }
                         }
                     }
                 }

diff --git a/topcat/src/main/uk/ac/starlink/topcat/plot2/FormStylePanel.java b/topcat/src/main/uk/ac/starlink/topcat/plot2/FormStylePanel.java
@@ -22,8 +22,10 @@
 import javax.swing.border.BevelBorder;
 import javax.swing.event.ListDataEvent;
 import javax.swing.event.ListDataListener;
+import uk.ac.starlink.ttools.plot.Style;
 import uk.ac.starlink.ttools.plot2.PlotUtil;
 import uk.ac.starlink.ttools.plot2.Plotter;
+import uk.ac.starlink.ttools.plot2.config.ConfigException;
 import uk.ac.starlink.ttools.plot2.config.ConfigKey;
 import uk.ac.starlink.ttools.plot2.config.ConfigMap;
 import uk.ac.starlink.topcat.ActionForwarder;
@@ -300,14 +302,21 @@ private void setSubsetSpecifierActive( boolean isActive ) {
      * the state of its configuration.
      */
     private void updateLegendIcon() {
-        final Icon icon;
+        Style style;
         if ( subsetSelector_.getSelectedItem() instanceof RowSubset ) {
             ConfigMap config = subsetSpecifier_.getSpecifiedValue();
-            icon = plotterFact_.getItem().createStyle( config ).getLegendIcon();
+            try {
+                style = plotterFact_.getItem().createStyle( config );
+            }
+            catch ( ConfigException e ) {
+                style = null;
+            }
         }
         else {
-            icon = IconUtils.emptyIcon( 24, 24 );
+            style = null;
         }
+        Icon icon = style == null ? IconUtils.emptyIcon( 24, 24 )
+                                  : style.getLegendIcon();
         iconLabel_.setIcon( icon );
     }
 

diff --git a/topcat/src/main/uk/ac/starlink/topcat/plot2/PlotPositionSpecifier.java b/topcat/src/main/uk/ac/starlink/topcat/plot2/PlotPositionSpecifier.java
@@ -112,7 +112,7 @@ private static ConfigKey<Integer> createIntegerKey( ConfigMeta meta ) {
             public String valueToString( Integer value ) {
                 return value == null ? "" : value.toString();
             }
-            public Integer stringToValue( String txt ) {
+            public Integer stringToValue( String txt ) throws ConfigException {
                 if ( txt == null || txt.trim().length() == 0 ) {
                     return null;
                 }

diff --git a/ttools/src/main/uk/ac/starlink/ttools/plot2/SurfaceFactory.java b/ttools/src/main/uk/ac/starlink/ttools/plot2/SurfaceFactory.java
@@ -54,7 +54,7 @@ public interface SurfaceFactory<P,A> {
      * @param  config  map of profile configuration items
      * @return  factory-specific plot surface profile
      */
-    P createProfile( ConfigMap config ) throws ConfigException;
+    P createProfile( ConfigMap config );
 
     /**
      * Returns the configuration keys that may be used to configure aspect
@@ -109,8 +109,7 @@ public interface SurfaceFactory<P,A> {
      * @param  ranges  range data filled in from layers, or null
      * @return   new aspect
      */
-    A createAspect( P profile, ConfigMap aspectConfig, Range[] ranges )
-        throws ConfigException;
+    A createAspect( P profile, ConfigMap aspectConfig, Range[] ranges );
 
     /**
      * Returns the configuration keys that may be used to configure

diff --git a/ttools/src/main/uk/ac/starlink/ttools/plot2/config/BooleanConfigKey.java b/ttools/src/main/uk/ac/starlink/ttools/plot2/config/BooleanConfigKey.java
@@ -45,7 +45,7 @@ public BooleanConfigKey( ConfigMeta meta ) {
         this( meta, false );
     }
 
-    public Boolean stringToValue( String txt ) {
+    public Boolean stringToValue( String txt ) throws ConfigException {
         txt = txt.toLowerCase();
         if ( TRUE_STRINGS.contains( txt ) ) {
             return Boolean.TRUE;

diff --git a/ttools/src/main/uk/ac/starlink/ttools/plot2/config/ChoiceConfigKey.java b/ttools/src/main/uk/ac/starlink/ttools/plot2/config/ChoiceConfigKey.java
@@ -90,7 +90,7 @@ public Map<String,T> getOptionMap() {
      */
     public abstract String stringifyValue( T value );
 
-    public T stringToValue( String sval ) {
+    public T stringToValue( String sval ) throws ConfigException {
         if ( sval == null || sval.length() == 0 ) {
             if ( nullPermitted_ ) {
                 return null; 

diff --git a/ttools/src/main/uk/ac/starlink/ttools/plot2/config/ConfigException.java b/ttools/src/main/uk/ac/starlink/ttools/plot2/config/ConfigException.java
@@ -3,15 +3,10 @@
 /**
  * Exception thrown when a configuration input value is not suitable.
  *
- * <p>This exception is currently unchecked because it will not
- * often be thrown and to avoid clutter in the code.
- * However, it should be checked for (explicitly caught) in some places,
- * in particular where controlled values are input from text fields.
- *
  * @author   Mark Taylor
  * @since    26 Feb 2013
  */
-public class ConfigException extends RuntimeException {
+public class ConfigException extends Exception {
 
     private final ConfigKey<?> key_;
 

diff --git a/ttools/src/main/uk/ac/starlink/ttools/plot2/config/DoubleConfigKey.java b/ttools/src/main/uk/ac/starlink/ttools/plot2/config/DoubleConfigKey.java
@@ -35,7 +35,7 @@ public String valueToString( Double value ) {
              : value.toString();
     }
 
-    public Double stringToValue( String txt ) {
+    public Double stringToValue( String txt ) throws ConfigException {
         if ( txt == null || txt.trim().length() == 0 ) {
             return new Double( Double.NaN );
         }

diff --git a/ttools/src/main/uk/ac/starlink/ttools/plot2/config/OptionConfigKey.java b/ttools/src/main/uk/ac/starlink/ttools/plot2/config/OptionConfigKey.java
@@ -81,7 +81,7 @@ public String valueToString( T value ) {
      * This means that if <code>valueToString</code> is overridden it
      * is usually not necessary to override this method.
      */
-    public T stringToValue( String txt ) {
+    public T stringToValue( String txt ) throws ConfigException {
         if ( txt == null || txt.trim().length() == 0 ) {
             return null;
         }

diff --git a/ttools/src/main/uk/ac/starlink/ttools/plot2/config/SkySysConfigKey.java b/ttools/src/main/uk/ac/starlink/ttools/plot2/config/SkySysConfigKey.java
@@ -24,7 +24,7 @@ public String valueToString( SkySys sys ) {
         return sys == null ? "generic" : super.valueToString( sys );
     }
 
-    public SkySys stringToValue( String str ) {
+    public SkySys stringToValue( String str ) throws ConfigException {
         return "generic".equals( str ) ? null
                                        : super.stringToValue( str );
     }

diff --git a/ttools/src/main/uk/ac/starlink/ttools/plot2/config/SubrangeConfigKey.java b/ttools/src/main/uk/ac/starlink/ttools/plot2/config/SubrangeConfigKey.java
@@ -53,7 +53,7 @@ public String valueToString( Subrange value ) {
         return format( value.getLow(), 3 ) + "," + format( value.getHigh(), 3 );
     }
 
-    public Subrange stringToValue( String txt ) {
+    public Subrange stringToValue( String txt ) throws ConfigException {
         String[] limits = txt.split( "," );
         if ( limits.length == 2 ) {
             String slo = limits[ 0 ].trim();

diff --git a/ttools/src/main/uk/ac/starlink/ttools/plot2/config/TimeConfigKey.java b/ttools/src/main/uk/ac/starlink/ttools/plot2/config/TimeConfigKey.java
@@ -59,7 +59,7 @@ public String valueToString( Double value ) {
                                      : formatTime( stime );
     }
 
-    public Double stringToValue( String txt ) {
+    public Double stringToValue( String txt ) throws ConfigException {
         if ( txt == null || txt.trim().length() == 0 ) {
             return new Double( Double.NaN );
         }

diff --git a/ttools/src/main/uk/ac/starlink/ttools/plot2/geom/CubeSurfaceFactory.java b/ttools/src/main/uk/ac/starlink/ttools/plot2/geom/CubeSurfaceFactory.java
@@ -312,7 +312,7 @@ public ConfigKey[] getProfileKeys() {
         return list.toArray( new ConfigKey[ 0 ] );
     }
 
-    public Profile createProfile( ConfigMap config ) throws ConfigException {
+    public Profile createProfile( ConfigMap config ) {
         boolean xlog = isIso_ ? false : config.get( XLOG_KEY );
         boolean ylog = isIso_ ? false : config.get( YLOG_KEY );
         boolean zlog = isIso_ ? false : config.get( ZLOG_KEY );

diff --git a/ttools/src/main/uk/ac/starlink/ttools/plot2/geom/PlaneSurfaceFactory.java b/ttools/src/main/uk/ac/starlink/ttools/plot2/geom/PlaneSurfaceFactory.java
@@ -179,7 +179,7 @@ public ConfigKey[] getProfileKeys() {
         return list.toArray( new ConfigKey[ 0 ] );
     }
 
-    public Profile createProfile( ConfigMap config ) throws ConfigException {
+    public Profile createProfile( ConfigMap config ) {
         boolean xlog = config.get( XLOG_KEY );
         boolean ylog = config.get( YLOG_KEY );
         boolean xflip = config.get( XFLIP_KEY );