Ready for release 2.4.4

pom.xml

@@ -8,7 +8,7 @@
     <artifactId>mapcode</artifactId>
 
     <packaging>jar</packaging>
-    <version>2.4.3</version>
+    <version>2.4.4</version>
 
     <name>Mapcode Java Library</name>
     <description>
@@ -36,7 +36,7 @@
             <name>Rijn Buve</name>
             <organization>Mapcode Foundation</organization>
             <roles>
-                <role>Managing Director, development</role>
+                <role>Managing Director, main contributor</role>
             </roles>
         </developer>
 

src/main/java/com/mapcode/Boundary.java

@@ -26,32 +26,28 @@
  * This class handles territory rectangles for mapcodes.
  */
 class Boundary {
-    private int lonMicroDegMin;     // Minimum longitude (in microdegrees). Inclusive.
-    private int lonMicroDegMax;     // Maximum longitude (in microdegrees). Exclusive.
     private int latMicroDegMin;     // Minimum latitude (in microdegrees). Inclusive.
+    private int lonMicroDegMin;     // Minimum longitude (in microdegrees). Inclusive.
     private int latMicroDegMax;     // Minimum latitude (in microdegrees). Exclusive.
+    private int lonMicroDegMax;     // Maximum longitude (in microdegrees). Exclusive.
 
+    // Get the singleton for the data model.
     private static final DataModel DATA_MODEL = DataModel.getInstance();
 
-    private Boundary(
-            final int lonMicroDegMin,
-            final int lonMicroDegMax,
-            final int latMicroDegMin,
-            final int latMicroDegMax) {
+    private Boundary(final int latMicroDegMin, final int lonMicroDegMin, final int latMicroDegMax, final int lonMicroDegMax) {
         this.lonMicroDegMin = lonMicroDegMin;
         this.latMicroDegMin = latMicroDegMin;
-        this.lonMicroDegMax = lonMicroDegMax;
         this.latMicroDegMax = latMicroDegMax;
+        this.lonMicroDegMax = lonMicroDegMax;
     }
 
     // You have to use this factory method instead of a ctor.
     @Nonnull
-    static Boundary createFromTerritoryRecord(final int territoryRecord) {
+    static Boundary createBoundaryForTerritoryRecord(final int territoryRecord) {
         return new Boundary(
-                DATA_MODEL.getLonMicroDegMin(territoryRecord),
-                DATA_MODEL.getLonMicroDegMax(territoryRecord),
-                DATA_MODEL.getLatMicroDegMin(territoryRecord),
-                DATA_MODEL.getLatMicroDegMax(territoryRecord));
+                DATA_MODEL.getLatMicroDegMin(territoryRecord), DATA_MODEL.getLonMicroDegMin(territoryRecord),
+                DATA_MODEL.getLatMicroDegMax(territoryRecord), DATA_MODEL.getLonMicroDegMax(territoryRecord)
+        );
     }
 
     int getLonMicroDegMin() {
@@ -98,7 +94,7 @@ boolean containsPoint(@Nonnull final Point p) {
         }
         final int lonMicroDeg = p.getLonMicroDeg();
 
-        // Longitude boundaries can extend (slightly) outside the [-180,180) range
+        // Longitude boundaries can extend (slightly) outside the [-180,180) range.
         if (lonMicroDeg < lonMicroDegMin) {
             return (lonMicroDegMin <= (lonMicroDeg + Point.MICRO_DEG_360)) && ((lonMicroDeg + Point.MICRO_DEG_360) < lonMicroDegMax);
         } else if (lonMicroDeg >= lonMicroDegMax) {

src/main/java/com/mapcode/CheckArgs.java

@@ -23,10 +23,10 @@
 
 /**
  * ----------------------------------------------------------------------------------------------
- * Package private implementation class. For internal use within the Mapcode implementation only.
+ * Package private implementation class. For internal use within the mapcode implementation only.
+ * ----------------------------------------------------------------------------------------------
  *
  * This class provides a number of helper methods to check (runtime) arguments.
- * ----------------------------------------------------------------------------------------------
  */
 @SuppressWarnings("OverlyBroadThrowsClause")
 class CheckArgs {

src/main/java/com/mapcode/Common.java

@@ -29,16 +29,23 @@
 class Common {
     private static final Logger LOG = LoggerFactory.getLogger(Common.class);
 
-    static final int[] nc = {
+    // TODO: Need better name and explanation.
+    static final int[] NC = {
             1, 31, 961, 29791, 923521, 28629151, 887503681
     };
-    static final int[] xSide = {
+
+    // TODO: Need better name and explanation.
+    static final int[] X_SIDE = {
             0, 5, 31, 168, 961, 5208, 29791, 165869, 923521, 5141947
     };
-    static final int[] ySide = {
+
+    // TODO: Need better name and explanation.
+    static final int[] Y_SIDE = {
             0, 6, 31, 176, 961, 5456, 29791, 165869, 923521, 5141947
     };
-    private static final int[] xDivider19 = {
+
+    // TODO: Need better name and explanation.
+    private static final int[] X_DIVIDER_19 = {
             360, 360, 360, 360, 360, 360, 361, 361, 361, 361,
             362, 362, 362, 363, 363, 363, 364, 364, 365, 366,
             366, 367, 367, 368, 369, 370, 370, 371, 372, 373,
@@ -66,66 +73,61 @@ private Common() {
     static {
 
         // This code shows a message when assertions are active or disabled. It (ab)uses assert for that...
+        // Some of the methods (and tests) take considerably longer with assertions checking, so it's useful
+        // to have this information in the log file.
+
         //noinspection UnusedAssignment
         boolean debug = false;
         //noinspection AssertWithSideEffects
         assert debug = true;
         //noinspection ConstantConditions
         if (debug) {
             LOG.info("Common: assertions are active (JVM runtime option '-ea')");
-        }
-        else {
+        } else {
             LOG.debug("Common: assertions are not active, they are bypassed");
         }
     }
 
-    /**
-     * This method returns a divider for longitude (multiplied by 4), for a given latitude.
-     *
-     * @param minY Latitude.
-     * @param maxY Longitude.
-     * @return Divider.
-     */
-    static int xDivider(final int minY, final int maxY) {
-        assert minY < maxY;
-        if (minY >= 0) {
-            // maxY > minY > 0
-            assert (maxY > minY) && (minY > 0);
-            return xDivider19[minY >> 19];
-        } else if (maxY >= 0) {
-            // maxY > 0 > minY
-            assert (maxY > 0) && (0 > minY);
-            return xDivider19[0];
+    // This method returns a divider for longitude (multiplied by 4), for a given latitude.
+    // TODO: Need better names for minY and maxY.
+    static int xDivider(final int latMin, final int latMax) {
+        assert latMin < latMax;
+        if (latMin >= 0) {
+            assert (latMax > latMin) && (latMin > 0);
+            return X_DIVIDER_19[latMin >> 19];
+        } else if (latMax >= 0) {
+            assert (latMax > 0) && (0 > latMin);
+            return X_DIVIDER_19[0];
         } else {
-            // 0 > maxY > minY
-            assert (0 > maxY) && (maxY > minY);
-            return xDivider19[(-maxY) >> 19];
+            assert (0 > latMax) && (latMax > latMin);
+            return X_DIVIDER_19[(-latMax) >> 19];
         }
     }
 
+    // TODO: Need to explain what a codex is.
     static int countCityCoordinatesForCountry(final int codex, final int territoryRecord, final int firstTerritoryRecord) {
         assert codex >= 0;
         assert territoryRecord >= 0;
         assert firstTerritoryRecord >= 0;
-        final int i = getFirstNamelessRecord(codex, territoryRecord, firstTerritoryRecord);
-        int e = territoryRecord;
-        while (Data.getCodex(e) == codex) {
-            e++;
+        final int firstRecord = getFirstNamelessRecord(codex, territoryRecord, firstTerritoryRecord);
+        int record = territoryRecord;
+        while (Data.getCodex(record) == codex) {
+            record++;
         }
-        assert i <= e;
-        return e - i;
+        assert firstRecord <= record;
+        return record - firstRecord;
     }
 
     static int getFirstNamelessRecord(final int codex, final int territoryRecord, final int firstTerritoryRecord) {
         assert codex >= 0;
         assert territoryRecord >= 0;
         assert firstTerritoryRecord >= 0;
-        int i = territoryRecord;
-        while ((i >= firstTerritoryRecord) && Data.isNameless(i) && (Data.getCodex(i) == codex)) {
-            i--;
+        int record = territoryRecord;
+        while ((record >= firstTerritoryRecord) && Data.isNameless(record) && (Data.getCodex(record) == codex)) {
+            record--;
         }
-        i++;
-        assert i <= territoryRecord;
-        return i;
+        record++;
+        assert record <= territoryRecord;
+        return record;
     }
 }

src/main/java/com/mapcode/Data.java

@@ -26,39 +26,49 @@
  * This class the data class for Mapcode codex items.
  */
 class Data {
+
+    // TODO: Need explanation what this is and how this is used.
     static final char[] ENCODE_CHARS = {
             '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',           // Numerals.
             'B', 'C', 'D', 'F', 'G', 'H', 'J', 'K', 'L', 'M',           // Consonants.
             'N', 'P', 'Q', 'R', 'S', 'T', 'V', 'W', 'X', 'Y', 'Z',
             'A', 'E', 'U'                                               // Vowels.
     };
 
+    // Get direct access to the data model.
     private static final DataModel DATA_MODEL = DataModel.getInstance();
 
     private Data() {
         // Disabled.
     }
 
+    // TODO: Explain what these values are. Can we make this an enum instead (safer)?
+    static final int TERRITORY_RECORD_TYPE_NONE = 0;
+    static final int TERRITORY_RECORD_TYPE_PIPE = 1;
+    static final int TERRITORY_RECORD_TYPE_PLUS = 2;
+    static final int TERRITORY_RECORD_TYPE_STAR = 3;
+
+    // TODO: Need to explain what "nameless" means and what a territoryRecord is (different from territoryNumber).
     static boolean isNameless(final int territoryRecord) {
         assert (0 <= territoryRecord) && (territoryRecord < DATA_MODEL.getNrTerritoryRecords());
         return (DATA_MODEL.getDataFlags(territoryRecord) & 64) != 0;
     }
 
+    // TODO: Need to explain what "special shape" means.
     static boolean isSpecialShape(final int territoryRecord) {
         assert (0 <= territoryRecord) && (territoryRecord < DATA_MODEL.getNrTerritoryRecords());
+
+        // TODO: The "magic" of binary operators and bit shifting should be in class DataModel, not here.
         return (DATA_MODEL.getDataFlags(territoryRecord) & 1024) != 0;
     }
 
-    static final int TERRITORY_RECORD_TYPE_NONE = 0;
-    static final int TERRITORY_RECORD_TYPE_PIPE = 1;
-    static final int TERRITORY_RECORD_TYPE_PLUS = 2;
-    static final int TERRITORY_RECORD_TYPE_STAR = 3;
-
+    // TODO: Explain what territory record types are. Can they be an enum instead?
     static int getTerritoryRecordType(final int territoryRecord) {
         assert (0 <= territoryRecord) && (territoryRecord < DATA_MODEL.getNrTerritoryRecords());
         return (DATA_MODEL.getDataFlags(territoryRecord) >> 7) & 3; // 1=pipe 2=plus 3=star
     }
 
+    // TODO: Explain what "restricted" means.
     static boolean isRestricted(final int territoryRecord) {
         assert (0 <= territoryRecord) && (territoryRecord < DATA_MODEL.getNrTerritoryRecords());
         return (DATA_MODEL.getDataFlags(territoryRecord) & 512) != 0;
@@ -70,17 +80,15 @@ static int getCodex(final int territoryRecord) {
         return (10 * (codexflags / 5)) + (codexflags % 5) + 1;
     }
 
+    // TODO: What does this method do? What is parameter i (rename)?
     @Nonnull
     static String headerLetter(final int i) {
         final int flags = DATA_MODEL.getDataFlags(i);
+
+        // TODO: The "magic" of how to interpret flags must be in DataModel, not here.
         if (((flags >> 7) & 3) == 1) {
             return Character.toString(ENCODE_CHARS[(flags >> 11) & 31]);
         }
         return "";
     }
-
-    @Nonnull
-    static Boundary getBoundary(final int territoryRecord) {
-        return Boundary.createFromTerritoryRecord(territoryRecord);
-    }
 }

src/main/java/com/mapcode/DataModel.java

@@ -34,6 +34,7 @@
 class DataModel {
     private static final Logger LOG = LoggerFactory.getLogger(DataModel.class);
 
+    // TODO: This class needs a thorough description of what the data file format looks like and what all bit fields means exactly.
     private static final int HEADER_ID_1 = 0;
     private static final int HEADER_ID_2 = 1;
     private static final int HEADER_VERSION_LO = 2;
@@ -196,11 +197,13 @@ int getNrTerritories() {
      *
      * @return Number of rectangles per territory.
      */
+    // TODO: Explain what territory records contain exactly.
     int getNrTerritoryRecords() {
         return nrTerritoryRecords;
     }
 
     @SuppressWarnings("PointlessArithmeticExpression")
+    // TODO: Explain what this does exactly, why not return a Point or Rectangle?
     int getLonMicroDegMin(final int territoryRecord) {
         return data[((territoryRecord * DATA_FIELDS_PER_REC) + POS_DATA_LON_MICRO_DEG_MIN)];
     }
@@ -221,17 +224,21 @@ int getDataFlags(final int territoryRecord) {
         return data[(territoryRecord * DATA_FIELDS_PER_REC) + POS_DATA_DATA_FLAGS] & MASK_DATA_DATA_FLAGS;
     }
 
+    // TODO: Explain what a "div" and "smart div" is and how you use, and why you need to use it.
     int getSmartDiv(final int territoryRecord) {
         return data[(territoryRecord * DATA_FIELDS_PER_REC) + POS_DATA_DATA_FLAGS] >> SHIFT_POS_DATA_SMART_DIV;
     }
 
+    // TODO: Explain what these methods do exactly.
     // Low-level routines for data access.
     @SuppressWarnings("PointlessArithmeticExpression")
     int getDataFirstRecord(final int territoryNumber) {
+        assert (0 <= territoryNumber) && (territoryNumber <= Territory.AAA.getNumber());
         return index[territoryNumber + POS_INDEX_FIRST_RECORD];
     }
 
     int getDataLastRecord(final int territoryNumber) {
+        assert (0 <= territoryNumber) && (territoryNumber <= Territory.AAA.getNumber());
         return index[territoryNumber + POS_INDEX_LAST_RECORD] - 1;
     }
 }