From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Fri, 12 Jan 2018 11:35:58 +0000 (+0000)
Subject: JAL-2876 some Checkstyle and other review comments addressed
X-Git-Url: http://source.jalview.org/gitweb/?a=commitdiff_plain;h=94202c5a8e9ddccf471659d63025f6174811df7f;p=jalview.git

JAL-2876 some Checkstyle and other review comments addressed
---

diff --git a/src/jalview/api/FeatureColourI.java b/src/jalview/api/FeatureColourI.java
index 18eb008..8b7b374 100644
--- a/src/jalview/api/FeatureColourI.java
+++ b/src/jalview/api/FeatureColourI.java
@@ -156,7 +156,7 @@ public interface FeatureColourI
    * @param feature
    * @return
    */
-  Color getColor(SequenceFeature feature);
+  Color getColour(SequenceFeature feature);
 
   /**
    * Update the min-max range for a graduated colour scheme
diff --git a/src/jalview/renderer/seqfeatures/FeatureRenderer.java b/src/jalview/renderer/seqfeatures/FeatureRenderer.java
index 1f47da3..d14571b 100644
--- a/src/jalview/renderer/seqfeatures/FeatureRenderer.java
+++ b/src/jalview/renderer/seqfeatures/FeatureRenderer.java
@@ -308,7 +308,7 @@ public class FeatureRenderer extends FeatureRendererModel
 
       for (SequenceFeature sf : overlaps)
       {
-        Color featureColour = fc.getColor(sf);
+        Color featureColour = fc.getColour(sf);
         if (featureColour == null)
         {
           // score feature outwith threshold for colouring
diff --git a/src/jalview/schemes/FeatureColour.java b/src/jalview/schemes/FeatureColour.java
index 17d5d6b..2007f8b 100644
--- a/src/jalview/schemes/FeatureColour.java
+++ b/src/jalview/schemes/FeatureColour.java
@@ -33,13 +33,23 @@ import java.util.StringTokenizer;
  */
 public class FeatureColour implements FeatureColourI
 {
-  private static final String BAR = "|";
+  private static final float _255F = 255f;
 
-  final private Color colour;
+  private static final String ABOVE = "above";
 
-  final private Color minColour;
+  private static final String BELOW = "below";
 
-  final private Color maxColour;
+  private static final String ABSO = "abso";
+
+  private static final String LABEL = "label";
+
+  private static final String PIPE = "|";
+
+  private final Color colour;
+
+  private final Color minColour;
+
+  private final Color maxColour;
 
   private boolean graduatedColour;
 
@@ -61,17 +71,118 @@ public class FeatureColour implements FeatureColourI
 
   private boolean autoScaled;
 
-  final private float minRed;
+  private final float minRed;
+
+  private final float minGreen;
+
+  private final float minBlue;
 
-  final private float minGreen;
+  private final float deltaRed;
 
-  final private float minBlue;
+  private final float deltaGreen;
+
+  private final float deltaBlue;
+
+  /**
+   * Default constructor
+   */
+  public FeatureColour()
+  {
+    this((Color) null);
+  }
+
+  /**
+   * Constructor given a simple colour
+   * 
+   * @param c
+   */
+  public FeatureColour(Color c)
+  {
+    minColour = Color.WHITE;
+    maxColour = Color.BLACK;
+    minRed = 0f;
+    minGreen = 0f;
+    minBlue = 0f;
+    deltaRed = 0f;
+    deltaGreen = 0f;
+    deltaBlue = 0f;
+    colour = c;
+  }
 
-  final private float deltaRed;
+  /**
+   * Constructor given a colour range and a score range
+   * 
+   * @param low
+   * @param high
+   * @param min
+   * @param max
+   */
+  public FeatureColour(Color low, Color high, float min, float max)
+  {
+    graduatedColour = true;
+    colour = null;
+    minColour = low == null ? Color.WHITE : low;
+    maxColour = high == null ? Color.BLACK : high;
+    threshold = Float.NaN;
+    isHighToLow = min >= max;
+    minRed = minColour.getRed() / _255F;
+    minGreen = minColour.getGreen() / _255F;
+    minBlue = minColour.getBlue() / _255F;
+    deltaRed = (maxColour.getRed() / _255F) - minRed;
+    deltaGreen = (maxColour.getGreen() / _255F) - minGreen;
+    deltaBlue = (maxColour.getBlue() / _255F) - minBlue;
+    if (isHighToLow)
+    {
+      base = max;
+      range = min - max;
+    }
+    else
+    {
+      base = min;
+      range = max - min;
+    }
+  }
 
-  final private float deltaGreen;
+  /**
+   * Copy constructor
+   * 
+   * @param fc
+   */
+  public FeatureColour(FeatureColour fc)
+  {
+    graduatedColour = fc.graduatedColour;
+    colour = fc.colour;
+    minColour = fc.minColour;
+    maxColour = fc.maxColour;
+    minRed = fc.minRed;
+    minGreen = fc.minGreen;
+    minBlue = fc.minBlue;
+    deltaRed = fc.deltaRed;
+    deltaGreen = fc.deltaGreen;
+    deltaBlue = fc.deltaBlue;
+    base = fc.base;
+    range = fc.range;
+    isHighToLow = fc.isHighToLow;
+    setAboveThreshold(fc.isAboveThreshold());
+    setBelowThreshold(fc.isBelowThreshold());
+    setThreshold(fc.getThreshold());
+    setAutoScaled(fc.isAutoScaled());
+    setColourByLabel(fc.isColourByLabel());
+  }
 
-  final private float deltaBlue;
+  /**
+   * Copy constructor with new min/max ranges
+   * 
+   * @param fc
+   * @param min
+   * @param max
+   */
+  public FeatureColour(FeatureColour fc, float min, float max)
+  {
+    this(fc);
+    graduatedColour = true;
+    updateBounds(min, max);
+  }
 
   /**
    * Parses a Jalview features file format colour descriptor
@@ -96,20 +207,20 @@ public class FeatureColour implements FeatureColourI
    */
   public static FeatureColour parseJalviewFeatureColour(String descriptor)
   {
-    StringTokenizer gcol = new StringTokenizer(descriptor, "|", true);
+    StringTokenizer gcol = new StringTokenizer(descriptor, PIPE, true);
     float min = Float.MIN_VALUE;
     float max = Float.MAX_VALUE;
     boolean labelColour = false;
 
     String mincol = gcol.nextToken();
-    if (mincol == "|")
+    if (mincol == PIPE)
     {
       throw new IllegalArgumentException(
               "Expected either 'label' or a colour specification in the line: "
                       + descriptor);
     }
     String maxcol = null;
-    if (mincol.toLowerCase().indexOf("label") == 0)
+    if (mincol.toLowerCase().indexOf(LABEL) == 0)
     {
       labelColour = true;
       mincol = (gcol.hasMoreTokens() ? gcol.nextToken() : null);
@@ -136,11 +247,13 @@ public class FeatureColour implements FeatureColourI
      * autoScaled == false ('abso'): colours range over min/max range
      */
     boolean autoScaled = true;
-    String tok = null, minval, maxval;
+    String tok = null;
+    String minval;
+    String maxval;
     if (mincol != null)
     {
       // at least four more tokens
-      if (mincol.equals("|"))
+      if (mincol.equals(PIPE))
       {
         mincol = "";
       }
@@ -149,7 +262,7 @@ public class FeatureColour implements FeatureColourI
         gcol.nextToken(); // skip next '|'
       }
       maxcol = gcol.nextToken();
-      if (maxcol.equals("|"))
+      if (maxcol.equals(PIPE))
       {
         maxcol = "";
       }
@@ -159,7 +272,7 @@ public class FeatureColour implements FeatureColourI
       }
       tok = gcol.nextToken();
       gcol.nextToken(); // skip next '|'
-      if (tok.toLowerCase().startsWith("abso"))
+      if (tok.toLowerCase().startsWith(ABSO))
       {
         minval = gcol.nextToken();
         gcol.nextToken(); // skip next '|'
@@ -180,7 +293,7 @@ public class FeatureColour implements FeatureColourI
         {
           min = new Float(minval).floatValue();
         }
-      } catch (Exception e)
+      } catch (NumberFormatException e)
       {
         throw new IllegalArgumentException(
                 "Couldn't parse the minimum value for graduated colour ("
@@ -192,7 +305,7 @@ public class FeatureColour implements FeatureColourI
         {
           max = new Float(maxval).floatValue();
         }
-      } catch (Exception e)
+      } catch (NumberFormatException e)
       {
         throw new IllegalArgumentException(
                 "Couldn't parse the maximum value for graduated colour ("
@@ -219,16 +332,17 @@ public class FeatureColour implements FeatureColourI
       featureColour.setColourByLabel(labelColour);
       featureColour.setAutoScaled(autoScaled);
       // add in any additional parameters
-      String ttype = null, tval = null;
+      String ttype = null;
+      String tval = null;
       if (gcol.hasMoreTokens())
       {
         // threshold type and possibly a threshold value
         ttype = gcol.nextToken();
-        if (ttype.toLowerCase().startsWith("below"))
+        if (ttype.toLowerCase().startsWith(BELOW))
         {
           featureColour.setBelowThreshold(true);
         }
-        else if (ttype.toLowerCase().startsWith("above"))
+        else if (ttype.toLowerCase().startsWith(ABOVE))
         {
           featureColour.setAboveThreshold(true);
         }
@@ -248,7 +362,7 @@ public class FeatureColour implements FeatureColourI
           gcol.nextToken();
           tval = gcol.nextToken();
           featureColour.setThreshold(new Float(tval).floatValue());
-        } catch (Exception e)
+        } catch (NumberFormatException e)
         {
           System.err.println("Couldn't parse threshold value as a float: ("
                   + tval + ")");
@@ -260,7 +374,7 @@ public class FeatureColour implements FeatureColourI
                 "Ignoring additional tokens in parameters in graduated colour specification\n");
         while (gcol.hasMoreTokens())
         {
-          System.err.println("|" + gcol.nextToken());
+          System.err.println(PIPE + gcol.nextToken());
         }
         System.err.println("\n");
       }
@@ -271,115 +385,6 @@ public class FeatureColour implements FeatureColourI
     }
   }
 
-  /**
-   * Default constructor
-   */
-  public FeatureColour()
-  {
-    this((Color) null);
-  }
-
-  /**
-   * Constructor given a simple colour
-   * 
-   * @param c
-   */
-  public FeatureColour(Color c)
-  {
-    minColour = Color.WHITE;
-    maxColour = Color.BLACK;
-    minRed = 0f;
-    minGreen = 0f;
-    minBlue = 0f;
-    deltaRed = 0f;
-    deltaGreen = 0f;
-    deltaBlue = 0f;
-    colour = c;
-  }
-
-  /**
-   * Constructor given a colour range and a score range
-   * 
-   * @param low
-   * @param high
-   * @param min
-   * @param max
-   */
-  public FeatureColour(Color low, Color high, float min, float max)
-  {
-    if (low == null)
-    {
-      low = Color.white;
-    }
-    if (high == null)
-    {
-      high = Color.black;
-    }
-    graduatedColour = true;
-    colour = null;
-    minColour = low;
-    maxColour = high;
-    threshold = Float.NaN;
-    isHighToLow = min >= max;
-    minRed = low.getRed() / 255f;
-    minGreen = low.getGreen() / 255f;
-    minBlue = low.getBlue() / 255f;
-    deltaRed = (high.getRed() / 255f) - minRed;
-    deltaGreen = (high.getGreen() / 255f) - minGreen;
-    deltaBlue = (high.getBlue() / 255f) - minBlue;
-    if (isHighToLow)
-    {
-      base = max;
-      range = min - max;
-    }
-    else
-    {
-      base = min;
-      range = max - min;
-    }
-  }
-
-  /**
-   * Copy constructor
-   * 
-   * @param fc
-   */
-  public FeatureColour(FeatureColour fc)
-  {
-    graduatedColour = fc.graduatedColour;
-    colour = fc.colour;
-    minColour = fc.minColour;
-    maxColour = fc.maxColour;
-    minRed = fc.minRed;
-    minGreen = fc.minGreen;
-    minBlue = fc.minBlue;
-    deltaRed = fc.deltaRed;
-    deltaGreen = fc.deltaGreen;
-    deltaBlue = fc.deltaBlue;
-    base = fc.base;
-    range = fc.range;
-    isHighToLow = fc.isHighToLow;
-    setAboveThreshold(fc.isAboveThreshold());
-    setBelowThreshold(fc.isBelowThreshold());
-    setThreshold(fc.getThreshold());
-    setAutoScaled(fc.isAutoScaled());
-    setColourByLabel(fc.isColourByLabel());
-  }
-
-  /**
-   * Copy constructor with new min/max ranges
-   * 
-   * @param fc
-   * @param min
-   * @param max
-   */
-  public FeatureColour(FeatureColour fc, float min, float max)
-  {
-    this(fc);
-    graduatedColour = true;
-    updateBounds(min, max);
-  }
-
   @Override
   public boolean isGraduatedColour()
   {
@@ -535,7 +540,7 @@ public class FeatureColour implements FeatureColourI
    * @return
    */
   @Override
-  public Color getColor(SequenceFeature feature)
+  public Color getColour(SequenceFeature feature)
   {
     if (isSimpleColour())
     {
@@ -547,14 +552,15 @@ public class FeatureColour implements FeatureColourI
      * (for graduated colour or colour by label)
      */
     float scr = feature.getScore();
-    boolean naN = Float.isNaN(scr);
-    if (!naN && isAboveThreshold() && scr <= threshold)
+    boolean isNan = Float.isNaN(scr);
+    if (!isNan)
     {
-      return null;
-    }
-    if (!naN && isBelowThreshold() && scr >= threshold)
-    {
-      return null;
+      boolean isAbove = isAboveThreshold() && scr <= threshold;
+      boolean isBelow = !isNan && isBelowThreshold() && scr >= threshold;
+      if (isAbove || isBelow)
+      {
+        return null;
+      }
     }
 
     if (isColourByLabel())
@@ -562,32 +568,36 @@ public class FeatureColour implements FeatureColourI
       return ColorUtils.createColourFromName(feature.getDescription());
     }
 
-    /*
-     * Float.NaN is assigned minimum visible score colour
-     */
-    if (naN)
-    {
-      return getMinColour();
-    }
-    if (range == 0.0)
-    {
-      return getMaxColour();
-    }
-    float scl = (scr - base) / range;
-    if (isHighToLow)
+    Color result = null;
+
+    if (isNan)
     {
-      scl = -scl;
+      result = getMinColour();
     }
-    if (scl < 0f)
+    else if (range == 0.0)
     {
-      scl = 0f;
+      result = getMaxColour();
     }
-    if (scl > 1f)
+    else
     {
-      scl = 1f;
+      float scl = (scr - base) / range;
+      if (isHighToLow)
+      {
+        scl = -scl;
+      }
+      if (scl < 0f)
+      {
+        scl = 0f;
+      }
+      if (scl > 1f)
+      {
+        scl = 1f;
+      }
+      result = new Color(minRed + scl * deltaRed,
+              minGreen + scl * deltaGreen, minBlue + scl * deltaBlue);
     }
-    return new Color(minRed + scl * deltaRed, minGreen + scl * deltaGreen,
-            minBlue + scl * deltaBlue);
+
+    return result;
   }
 
   /**
@@ -639,32 +649,32 @@ public class FeatureColour implements FeatureColourI
       StringBuilder sb = new StringBuilder(32);
       if (isColourByLabel())
       {
-        sb.append("label");
+        sb.append(LABEL);
         if (hasThreshold())
         {
-          sb.append(BAR).append(BAR).append(BAR);
+          sb.append(PIPE).append(PIPE).append(PIPE);
         }
       }
       if (isGraduatedColour())
       {
-        sb.append(Format.getHexString(getMinColour())).append(BAR);
-        sb.append(Format.getHexString(getMaxColour())).append(BAR);
+        sb.append(Format.getHexString(getMinColour())).append(PIPE);
+        sb.append(Format.getHexString(getMaxColour())).append(PIPE);
         if (!isAutoScaled())
         {
-          sb.append("abso").append(BAR);
+          sb.append(ABSO).append(PIPE);
         }
       }
       if (hasThreshold() || isGraduatedColour())
       {
-        sb.append(getMin()).append(BAR);
-        sb.append(getMax()).append(BAR);
+        sb.append(getMin()).append(PIPE);
+        sb.append(getMax()).append(PIPE);
         if (isBelowThreshold())
         {
-          sb.append("below").append(BAR).append(getThreshold());
+          sb.append(BELOW).append(PIPE).append(getThreshold());
         }
         else if (isAboveThreshold())
         {
-          sb.append("above").append(BAR).append(getThreshold());
+          sb.append(ABOVE).append(PIPE).append(getThreshold());
         }
         else
         {
diff --git a/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java b/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java
index 2f30e94..ea4fbdf 100644
--- a/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java
+++ b/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java
@@ -570,7 +570,7 @@ public abstract class FeatureRendererModel
   public Color getColour(SequenceFeature feature)
   {
     FeatureColourI fc = getFeatureStyle(feature.getType());
-    return fc.getColor(feature);
+    return fc.getColour(feature);
   }
 
   /**
diff --git a/test/jalview/schemes/FeatureColourTest.java b/test/jalview/schemes/FeatureColourTest.java
index ed7adb8..85c3f8b 100644
--- a/test/jalview/schemes/FeatureColourTest.java
+++ b/test/jalview/schemes/FeatureColourTest.java
@@ -85,26 +85,26 @@ public class FeatureColourTest
   }
 
   @Test(groups = { "Functional" })
-  public void testGetColor_simpleColour()
+  public void testGetColour_simpleColour()
   {
     FeatureColour fc = new FeatureColour(Color.RED);
     assertEquals(Color.RED,
-            fc.getColor(new SequenceFeature("Cath", "", 1, 2, 0f, null)));
+            fc.getColour(new SequenceFeature("Cath", "", 1, 2, 0f, null)));
   }
 
   @Test(groups = { "Functional" })
-  public void testGetColor_colourByLabel()
+  public void testGetColour_colourByLabel()
   {
     FeatureColour fc = new FeatureColour();
     fc.setColourByLabel(true);
     SequenceFeature sf = new SequenceFeature("type", "desc", 0, 20, 1f,
             null);
     Color expected = ColorUtils.createColourFromName("desc");
-    assertEquals(expected, fc.getColor(sf));
+    assertEquals(expected, fc.getColour(sf));
   }
 
   @Test(groups = { "Functional" })
-  public void testGetColor_Graduated()
+  public void testGetColour_Graduated()
   {
     // graduated colour from score 0 to 100, gray(128, 128, 128) to red(255, 0,
     // 0)
@@ -117,11 +117,11 @@ public class FeatureColourTest
     float green = 128 / 255f + 3 / 4f * (0 - 128) / 255f;
     float blue = 128 / 255f + 3 / 4f * (0 - 128) / 255f;
     Color expected = new Color(red, green, blue);
-    assertEquals(expected, fc.getColor(sf));
+    assertEquals(expected, fc.getColour(sf));
   }
 
   @Test(groups = { "Functional" })
-  public void testGetColor_aboveBelowThreshold()
+  public void testGetColour_aboveBelowThreshold()
   {
     // gradient from [50, 150] from WHITE(255, 255, 255) to BLACK(0, 0, 0)
     FeatureColour fc = new FeatureColour(Color.WHITE, Color.BLACK, 50f,
@@ -136,20 +136,20 @@ public class FeatureColourTest
             Float.NaN, null);
 
     fc.setThreshold(100f); // ignore for now
-    assertEquals(new Color(204, 204, 204), fc.getColor(sf));
-    assertEquals(Color.white, fc.getColor(sf2));
+    assertEquals(new Color(204, 204, 204), fc.getColour(sf));
+    assertEquals(Color.white, fc.getColour(sf2));
 
     fc.setAboveThreshold(true); // feature lies below threshold
-    assertNull(fc.getColor(sf));
-    assertEquals(Color.white, fc.getColor(sf2));
+    assertNull(fc.getColour(sf));
+    assertEquals(Color.white, fc.getColour(sf2));
 
     fc.setBelowThreshold(true);
     fc.setThreshold(70f);
-    assertNull(fc.getColor(sf)); // feature score == threshold - hidden
-    assertEquals(Color.white, fc.getColor(sf2));
+    assertNull(fc.getColour(sf)); // feature score == threshold - hidden
+    assertEquals(Color.white, fc.getColour(sf2));
     fc.setThreshold(69f);
-    assertNull(fc.getColor(sf)); // feature score > threshold - hidden
-    assertEquals(Color.white, fc.getColor(sf2));
+    assertNull(fc.getColour(sf)); // feature score > threshold - hidden
+    assertEquals(Color.white, fc.getColour(sf2));
   }
 
   /**
@@ -342,7 +342,7 @@ public class FeatureColourTest
   }
 
   @Test(groups = { "Functional" })
-  public void testGetColor_colourByLabel_withThreshold()
+  public void testGetColour_colourByLabel_withThreshold()
   {
     FeatureColour fc = new FeatureColour();
     fc.setColourByLabel(true);
@@ -350,23 +350,23 @@ public class FeatureColourTest
             null);
     fc.setThreshold(0);
     Color expected = ColorUtils.createColourFromName("desc");
-    assertEquals(expected, fc.getColor(sf));
+    assertEquals(expected, fc.getColour(sf));
 
     // score (1f) is above threshold
     fc.setAboveThreshold(true);
-    assertEquals(expected, fc.getColor(sf));
+    assertEquals(expected, fc.getColour(sf));
 
     // score is not above threshold
     fc.setThreshold(2f);
-    assertNull(fc.getColor(sf));
+    assertNull(fc.getColour(sf));
 
     // score is not below threshold
     fc.setThreshold(0f);
     fc.setBelowThreshold(true);
-    assertNull(fc.getColor(sf));
+    assertNull(fc.getColour(sf));
 
     // score is below threshold
     fc.setThreshold(3f);
-    assertEquals(expected, fc.getColor(sf));
+    assertEquals(expected, fc.getColour(sf));
   }
 }