From 56da3dabe5fd0b8e68daca4ae742850bd763a207 Mon Sep 17 00:00:00 2001
From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Fri, 4 Jan 2019 08:19:36 +0000
Subject: [PATCH] JAL-3181 simplified code and test

---
 src/jalview/util/UrlLink.java      |   34 ++++++++++++++--------------------
 test/jalview/util/UrlLinkTest.java |    6 ++----
 2 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/src/jalview/util/UrlLink.java b/src/jalview/util/UrlLink.java
index dda9e0e..18ee9b6 100644
--- a/src/jalview/util/UrlLink.java
+++ b/src/jalview/util/UrlLink.java
@@ -46,11 +46,17 @@ import java.util.Vector;
  */
 public class UrlLink
 {
+  private static final String SEQUENCEID_PLACEHOLDER = DELIM + SEQUENCE_ID
+          + DELIM;
+
+  private static final String ACCESSION_PLACEHOLDER = DELIM + DB_ACCESSION
+          + DELIM;
+
   /**
    * A comparator that puts SEQUENCE_ID template links before DB_ACCESSION
-   * links, and otherwise orders by link name (not case sensitive). It expects
-   * to compare strings formatted as "Name|URLTemplate" where the template may
-   * include $SEQUENCE_ID$ or $DB_ACCESSION$ or neither.
+   * links, and otherwise orders by link name + url (not case sensitive). It
+   * expects to compare strings formatted as "Name|URLTemplate" where the
+   * template may include $SEQUENCE_ID$ or $DB_ACCESSION$ or neither.
    */
   public static final Comparator<String> LINK_COMPARATOR = new Comparator<String>()
   {
@@ -61,29 +67,17 @@ public class UrlLink
       {
         return 0; // for failsafe only
       }
-      String[] tokens1 = link1.split("\\|");
-      String[] tokens2 = link2.split("\\|");
-      if (tokens1.length < 2 || tokens2.length < 2)
-      {
-        // for failsafe only
-        return String.CASE_INSENSITIVE_ORDER.compare(link1, link2);
-      }
-      String name1 = tokens1[0];
-      String name2 = tokens2[0];
-      String pattern1 = tokens1[1];
-      String pattern2 = tokens2[1];
-      if (pattern1.contains(UrlConstants.SEQUENCE_ID)
-              && pattern2.contains(UrlConstants.DB_ACCESSION))
+      if (link1.contains(SEQUENCEID_PLACEHOLDER)
+              && link2.contains(ACCESSION_PLACEHOLDER))
       {
         return -1;
       }
-      if (pattern2.contains(UrlConstants.SEQUENCE_ID)
-              && pattern1.contains(UrlConstants.DB_ACCESSION))
+      if (link2.contains(SEQUENCEID_PLACEHOLDER)
+              && link1.contains(ACCESSION_PLACEHOLDER))
       {
         return 1;
       }
-      return String.CASE_INSENSITIVE_ORDER.compare(name1, name2);
-
+      return String.CASE_INSENSITIVE_ORDER.compare(link1, link2);
     }
   };
 
diff --git a/test/jalview/util/UrlLinkTest.java b/test/jalview/util/UrlLinkTest.java
index 81fe3d2..c7d37b0 100644
--- a/test/jalview/util/UrlLinkTest.java
+++ b/test/jalview/util/UrlLinkTest.java
@@ -450,11 +450,9 @@ public class UrlLinkTest
 
     /*
      * SEQUENCE_ID templates should come before DB_ACCESSION templates
-     * (note we are 'pretending' EMBL-EBI is a DB link so the test shows that
-     * these are not ordered by link name)
      */
-    String seqIdUrl = "Ensembl|https://www.ensembl.org/Multi/Search/Results?q=$SEQUENCE_ID$";
-    String dbRefUrl = "EBI|https://www.ebi.ac.uk/ebisearch/search.ebi?db=allebi&query=$DB_ACCESSION$";
+    String dbRefUrl = "Cath|http://www.cathdb.info/version/v4_2_0/superfamily/$DB_ACCESSION$";
+    String seqIdUrl = "EBI|https://www.ebi.ac.uk/ebisearch/search.ebi?db=allebi&query=$SEQUENCE_ID$";
     assertTrue(c.compare(seqIdUrl, dbRefUrl) < 0);
     assertTrue(c.compare(dbRefUrl, seqIdUrl) > 0);
 
-- 
1.7.10.2